* [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
@ 2024-12-17 19:57 Vadim Fedorenko
2024-12-18 3:52 ` Richard Cochran
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Vadim Fedorenko @ 2024-12-17 19:57 UTC (permalink / raw)
To: Vadim Fedorenko, Dragos Tatulea, Gal Pressman, Jakub Kicinski
Cc: Tariq Toukan, Carolina Jubran, Bar Shapira, netdev, Andrew Lunn,
Paolo Abeni, Richard Cochran, David S. Miller, Saeed Mahameed,
Vadim Fedorenko
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@meta.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 {
--
2.43.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2024-12-17 19:57 [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks Vadim Fedorenko
@ 2024-12-18 3:52 ` Richard Cochran
2024-12-18 10:19 ` Vadim Fedorenko
2024-12-22 14:10 ` Dragos Tatulea
2025-01-07 6:21 ` Tariq Toukan
2 siblings, 1 reply; 7+ messages in thread
From: Richard Cochran @ 2024-12-18 3:52 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: Vadim Fedorenko, Dragos Tatulea, Gal Pressman, Jakub Kicinski,
Tariq Toukan, Carolina Jubran, Bar Shapira, netdev, Andrew Lunn,
Paolo Abeni, David S. Miller, Saeed Mahameed
On Tue, Dec 17, 2024 at 11:57:38AM -0800, 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.
Yes, and you can configure that thread with chrt to ensure timely
invocation of the callback.
> @@ -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);
Do you need ptp_cancel_worker_sync() ?
Thanks,
Richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2024-12-18 3:52 ` Richard Cochran
@ 2024-12-18 10:19 ` Vadim Fedorenko
0 siblings, 0 replies; 7+ messages in thread
From: Vadim Fedorenko @ 2024-12-18 10:19 UTC (permalink / raw)
To: Richard Cochran
Cc: Dragos Tatulea, Gal Pressman, Jakub Kicinski, Tariq Toukan,
Carolina Jubran, Bar Shapira, netdev, Andrew Lunn, Paolo Abeni,
David S. Miller, Saeed Mahameed
On 18/12/2024 03:52, Richard Cochran wrote:
> On Tue, Dec 17, 2024 at 11:57:38AM -0800, 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.
>
> Yes, and you can configure that thread with chrt to ensure timely
> invocation of the callback.
>
>> @@ -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);
>
> Do you need ptp_cancel_worker_sync() ?
ptp_clock_unregister() calls kthread_cancel_delayed_work_sync() if
ptp->kworker exists, no need to call ptp_cancel_worker_sync() on cleanup.
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2024-12-17 19:57 [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks Vadim Fedorenko
2024-12-18 3:52 ` Richard Cochran
@ 2024-12-22 14:10 ` Dragos Tatulea
2025-01-03 22:25 ` Vadim Fedorenko
2025-01-07 6:21 ` Tariq Toukan
2 siblings, 1 reply; 7+ messages in thread
From: Dragos Tatulea @ 2024-12-22 14:10 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Gal Pressman, Jakub Kicinski
Cc: Tariq Toukan, Carolina Jubran, Bar Shapira, netdev, Andrew Lunn,
Paolo Abeni, Richard Cochran, David S. Miller, Saeed Mahameed
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@meta.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@nvidia.com>
Thanks,
Dragos
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2024-12-22 14:10 ` Dragos Tatulea
@ 2025-01-03 22:25 ` Vadim Fedorenko
2025-01-06 14:45 ` Tariq Toukan
0 siblings, 1 reply; 7+ messages in thread
From: Vadim Fedorenko @ 2025-01-03 22:25 UTC (permalink / raw)
To: Saeed Mahameed, Tariq Toukan, Leon Romanovsky
Cc: Carolina Jubran, Bar Shapira, netdev, Andrew Lunn, Paolo Abeni,
Richard Cochran, David S. Miller, Dragos Tatulea, Jakub Kicinski,
Gal Pressman
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@meta.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@nvidia.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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2025-01-03 22:25 ` Vadim Fedorenko
@ 2025-01-06 14:45 ` Tariq Toukan
0 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2025-01-06 14:45 UTC (permalink / raw)
To: Vadim Fedorenko, Saeed Mahameed, Tariq Toukan, Leon Romanovsky
Cc: Carolina Jubran, Bar Shapira, netdev, Andrew Lunn, Paolo Abeni,
Richard Cochran, David S. Miller, Dragos Tatulea, Jakub Kicinski,
Gal Pressman
On 04/01/2025 0:25, Vadim Fedorenko wrote:
> 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@meta.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@nvidia.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
>
>
Hi,
Back after a long vacation.
I'll reply shortly.
Regards,
Tariq
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks
2024-12-17 19:57 [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks Vadim Fedorenko
2024-12-18 3:52 ` Richard Cochran
2024-12-22 14:10 ` Dragos Tatulea
@ 2025-01-07 6:21 ` Tariq Toukan
2 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2025-01-07 6:21 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Dragos Tatulea, Gal Pressman,
Jakub Kicinski
Cc: Tariq Toukan, Carolina Jubran, Bar Shapira, netdev, Andrew Lunn,
Paolo Abeni, Richard Cochran, David S. Miller, Saeed Mahameed
On 17/12/2024 21:57, 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@meta.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");
Same-line strings are preferable (unless they're really
ugly/non-readable..).
They ease find/grep operations.
> + }
>
> 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 {
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-01-07 6:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-17 19:57 [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks Vadim Fedorenko
2024-12-18 3:52 ` Richard Cochran
2024-12-18 10:19 ` Vadim Fedorenko
2024-12-22 14:10 ` Dragos Tatulea
2025-01-03 22:25 ` Vadim Fedorenko
2025-01-06 14:45 ` Tariq Toukan
2025-01-07 6:21 ` Tariq Toukan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).