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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 18/10/2024 05:08, Rahul Rameshbabu wrote:
> On Mon, 14 Oct, 2024 10:01:03 -0700 Vadim Fedorenko <vadfed@meta.com> 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 <vadfed@meta.com>
>> ---
>>   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 <rrameshbabu@nvidia.com>

Rahul, Tariq,

will you take it through mlx5-next, or should it go directly to
net-next?

Thanks!