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

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

[Vadim Fedorenko]

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;
-- 
2.43.5

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

[Rahul Rameshbabu]

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>

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

[Vadim Fedorenko]

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!

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

[Tariq Toukan]

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

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 14 Oct 2024 10:01:03 -0700 you 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(-)

Here is the summary with links:
  - [net-next] mlx5_en: use read sequence for gettimex64
    https://git.kernel.org/netdev/net-next/c/0452a2d8b8b9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html