Subject: Re: [PATCH net-next] mlx5_en: use read sequence for gettimex64

From: Rahul Rameshbabu <[email-masked]>
Date: Thu, 17 Oct 2024 21:08:17 -0700

On Mon, 14 Oct, 2024 10:01:03 -0700 Vadim Fedorenko <[email-masked]> wrote:
> The gettimex64() doesn't modify values in timecounter, that's why there
> is no need to update sequence counter. Reduce the contention on sequence
> lock for multi-thread PHC reading use-case.
>
> Signed-off-by: Vadim Fedorenko <[email-masked]>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> index b306ae79bf97..4822d01123b4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> @@ -402,9 +402,7 @@ static int mlx5_ptp_gettimex(struct ptp_clock_info *ptp, struct timespec64 *ts,
>  			     struct ptp_system_timestamp *sts)
>  {
>  	struct mlx5_clock *clock = container_of(ptp, struct mlx5_clock, ptp_info);
> -	struct mlx5_timer *timer = &clock->timer;
>  	struct mlx5_core_dev *mdev;
> -	unsigned long flags;
>  	u64 cycles, ns;
>  
>  	mdev = container_of(clock, struct mlx5_core_dev, clock);
> @@ -413,10 +411,8 @@ static int mlx5_ptp_gettimex(struct ptp_clock_info *ptp, struct timespec64 *ts,
>  		goto out;
>  	}
>  
> -	write_seqlock_irqsave(&clock->lock, flags);
>  	cycles = mlx5_read_time(mdev, sts, false);
> -	ns = timecounter_cyc2time(&timer->tc, cycles);
> -	write_sequnlock_irqrestore(&clock->lock, flags);
> +	ns = mlx5_timecounter_cyc2time(clock, cycles);
>  	*ts = ns_to_timespec64(ns);
>  out:
>  	return 0;

The patch seems like a good cleanup to me. Like Vadim mentioned, we
should not need to update the timecounter since this simply a read
operation.

Reviewed-by: Rahul Rameshbabu <[email-masked]>