Re: [PATCH net-next] net/mlx5: use do_aux_work for PHC overflow checks

["Dragos Tatulea" <dtatulea@nvidia.com>]

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