[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6c9730b3-b35d-47d9-9cd6-ec2e05e4e8e5@linux.dev>
Date: Fri, 3 Jan 2025 22:25:44 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Saeed Mahameed <saeedm@...dia.com>, Tariq Toukan <tariqt@...dia.com>,
Leon Romanovsky <leon@...nel.org>
Cc: Carolina Jubran <cjubran@...dia.com>, Bar Shapira <bshapira@...dia.com>,
netdev@...r.kernel.org, Andrew Lunn <andrew+netdev@...n.ch>,
Paolo Abeni <pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>,
"David S. Miller" <davem@...emloft.net>, Dragos Tatulea
<dtatulea@...dia.com>, Jakub Kicinski <kuba@...nel.org>,
Gal Pressman <gal@...dia.com>
Subject: Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow
checks
On 22/12/2024 14:10, Dragos Tatulea wrote:
> On Tue Dec 17, 2024 at 8:57 PM CET, Vadim Fedorenko wrote:
>> The overflow_work is using system wq to do overflow checks and updates
>> for PHC device timecounter, which might be overhelmed by other tasks.
>> But there is dedicated kthread in PTP subsystem designed for such
>> things. This patch changes the work queue to proper align with PTP
>> subsystem and to avoid overloading system work queue.
>> The adjfine() function acts the same way as overflow check worker,
>> we can postpone ptp aux worker till the next overflow period after
>> adjfine() was called.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
>> ---
>> .../ethernet/mellanox/mlx5/core/lib/clock.c | 25 +++++++++++--------
>> include/linux/mlx5/driver.h | 1 -
>> 2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> index 4822d01123b4..ff3780331273 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> @@ -322,17 +322,16 @@ static void mlx5_pps_out(struct work_struct *work)
>> }
>> }
>>
>> -static void mlx5_timestamp_overflow(struct work_struct *work)
>> +static long mlx5_timestamp_overflow(struct ptp_clock_info *ptp_info)
>> {
>> - struct delayed_work *dwork = to_delayed_work(work);
>> struct mlx5_core_dev *mdev;
>> struct mlx5_timer *timer;
>> struct mlx5_clock *clock;
>> unsigned long flags;
>>
>> - timer = container_of(dwork, struct mlx5_timer, overflow_work);
>> - clock = container_of(timer, struct mlx5_clock, timer);
>> + clock = container_of(ptp_info, struct mlx5_clock, ptp_info);
>> mdev = container_of(clock, struct mlx5_core_dev, clock);
>> + timer = &clock->timer;
>>
>> if (mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)
>> goto out;
>> @@ -343,7 +342,7 @@ static void mlx5_timestamp_overflow(struct work_struct *work)
>> write_sequnlock_irqrestore(&clock->lock, flags);
>>
>> out:
>> - schedule_delayed_work(&timer->overflow_work, timer->overflow_period);
>> + return timer->overflow_period;
>> }
>>
>> static int mlx5_ptp_settime_real_time(struct mlx5_core_dev *mdev,
>> @@ -517,6 +516,7 @@ static int mlx5_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
>> timer->cycles.mult = mult;
>> mlx5_update_clock_info_page(mdev);
>> write_sequnlock_irqrestore(&clock->lock, flags);
>> + ptp_schedule_worker(clock->ptp, timer->overflow_period);
>>
>> return 0;
>> }
>> @@ -852,6 +852,7 @@ static const struct ptp_clock_info mlx5_ptp_clock_info = {
>> .settime64 = mlx5_ptp_settime,
>> .enable = NULL,
>> .verify = NULL,
>> + .do_aux_work = mlx5_timestamp_overflow,
>> };
>>
>> static int mlx5_query_mtpps_pin_mode(struct mlx5_core_dev *mdev, u8 pin,
>> @@ -1052,12 +1053,12 @@ static void mlx5_init_overflow_period(struct mlx5_clock *clock)
>> do_div(ns, NSEC_PER_SEC / HZ);
>> timer->overflow_period = ns;
>>
>> - INIT_DELAYED_WORK(&timer->overflow_work, mlx5_timestamp_overflow);
>> - if (timer->overflow_period)
>> - schedule_delayed_work(&timer->overflow_work, 0);
>> - else
>> + if (!timer->overflow_period) {
>> + timer->overflow_period = HZ;
>> mlx5_core_warn(mdev,
>> - "invalid overflow period, overflow_work is not scheduled\n");
>> + "invalid overflow period,"
>> + "overflow_work is scheduled once per second\n");
>> + }
>>
>> if (clock_info)
>> clock_info->overflow_period = timer->overflow_period;
>> @@ -1172,6 +1173,9 @@ void mlx5_init_clock(struct mlx5_core_dev *mdev)
>>
>> MLX5_NB_INIT(&clock->pps_nb, mlx5_pps_event, PPS_EVENT);
>> mlx5_eq_notifier_register(mdev, &clock->pps_nb);
>> +
>> + if (clock->ptp)
>> + ptp_schedule_worker(clock->ptp, 0);
>> }
>>
>> void mlx5_cleanup_clock(struct mlx5_core_dev *mdev)
>> @@ -1188,7 +1192,6 @@ void mlx5_cleanup_clock(struct mlx5_core_dev *mdev)
>> }
>>
>> cancel_work_sync(&clock->pps_info.out_work);
>> - cancel_delayed_work_sync(&clock->timer.overflow_work);
>>
>> if (mdev->clock_info) {
>> free_page((unsigned long)mdev->clock_info);
>> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
>> index fc7e6153b73d..3ac2fc1b52cf 100644
>> --- a/include/linux/mlx5/driver.h
>> +++ b/include/linux/mlx5/driver.h
>> @@ -690,7 +690,6 @@ struct mlx5_timer {
>> struct timecounter tc;
>> u32 nominal_c_mult;
>> unsigned long overflow_period;
>> - struct delayed_work overflow_work;
>> };
>>
>> struct mlx5_clock {
>
> Reviewed-by: Dragos Tatulea <dtatulea@...dia.com>
>
Hi Saeed, Tariq, Leon!
We need explicit Ack-by from official mlx5 maintainer here to let this
patch go directly to net-next. Could you please check it?
Thanks,
Vadim
Powered by blists - more mailing lists