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

[Tariq Toukan <ttoukan.linux@gmail.com>]

On 30/10/2024 12:17, Vadim Fedorenko wrote:
> 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!
> 

Hi,

Sorry for the late response, I missed this question previously.

Acked-by: Tariq Toukan <tariqt@nvidia.com>

Should go to net-next.

Regards,
Tariq