| Message ID | 20241014170103.2473580-1-vadfed@meta.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0452a2d8b8b98a5b1a9139c1a9ed98bccee356cc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] mlx5_en: use read sequence for gettimex64 | expand |
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>
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!
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
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!
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 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(-)