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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wmi6njda.fsf@nvidia.com>
Date: Thu, 17 Oct 2024 21:08:17 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: Vadim Fedorenko <vadfed@...a.com>
Cc: Saeed Mahameed <saeedm@...dia.com>,  Leon Romanovsky <leon@...nel.org>,
  Tariq Toukan <tariqt@...dia.com>,  "David S. Miller"
 <davem@...emloft.net>,  Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
  Jakub Kicinski <kuba@...nel.org>,  Eric Dumazet <edumazet@...gle.com>,
  <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] mlx5_en: use read sequence for gettimex64

On Mon, 14 Oct, 2024 10:01:03 -0700 Vadim Fedorenko <vadfed@...a.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@...a.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@...dia.com>