Re: [net-next V2 01/15] net/mlx5: Parallelize vhca event handling

[Jacob Keller <jacob.e.keller@intel.com>]

On 10/12/2023 12:27 PM, Saeed Mahameed wrote:
> From: Wei Zhang <weizhang@nvidia.com>
> 
> At present, mlx5 driver have a general purpose
> event handler which not only handles vhca event
> but also many other events. This incurs a huge
> bottleneck because the event handler is
> implemented by single threaded workqueue and all
> events are forced to be handled in serial manner
> even though application tries to create multiple
> SFs simultaneously.
> 
> Introduce a dedicated vhca event handler which
> manages SFs parallel creation.
> 
> Signed-off-by: Wei Zhang <weizhang@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> Reviewed-by: Shay Drory <shayd@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/events.c  |  5 --
>  .../ethernet/mellanox/mlx5/core/mlx5_core.h   |  3 +-
>  .../mellanox/mlx5/core/sf/vhca_event.c        | 57 ++++++++++++++++++-
>  include/linux/mlx5/driver.h                   |  1 +
>  4 files changed, 57 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/events.c b/drivers/net/ethernet/mellanox/mlx5/core/events.c
> index 3ec892d51f57..d91ea53eb394 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/events.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/events.c
> @@ -441,8 +441,3 @@ int mlx5_blocking_notifier_call_chain(struct mlx5_core_dev *dev, unsigned int ev
>  
>  	return blocking_notifier_call_chain(&events->sw_nh, event, data);
>  }
> -
> -void mlx5_events_work_enqueue(struct mlx5_core_dev *dev, struct work_struct *work)
> -{
> -	queue_work(dev->priv.events->wq, work);
> -}
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> index 124352459c23..94f809f52f27 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> @@ -143,6 +143,8 @@ enum mlx5_semaphore_space_address {
>  
>  #define MLX5_DEFAULT_PROF       2
>  #define MLX5_SF_PROF		3
> +#define MLX5_NUM_FW_CMD_THREADS 8
> +#define MLX5_DEV_MAX_WQS	MLX5_NUM_FW_CMD_THREADS
>  
>  static inline int mlx5_flexible_inlen(struct mlx5_core_dev *dev, size_t fixed,
>  				      size_t item_size, size_t num_items,
> @@ -331,7 +333,6 @@ int mlx5_vport_set_other_func_cap(struct mlx5_core_dev *dev, const void *hca_cap
>  #define mlx5_vport_get_other_func_general_cap(dev, vport, out)		\
>  	mlx5_vport_get_other_func_cap(dev, vport, out, MLX5_CAP_GENERAL)
>  
> -void mlx5_events_work_enqueue(struct mlx5_core_dev *dev, struct work_struct *work);
>  static inline u32 mlx5_sriov_get_vf_total_msix(struct pci_dev *pdev)
>  {
>  	struct mlx5_core_dev *dev = pci_get_drvdata(pdev);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/vhca_event.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/vhca_event.c
> index d908fba968f0..c6fd729de8b2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/vhca_event.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/vhca_event.c
> @@ -21,6 +21,15 @@ struct mlx5_vhca_event_work {
>  	struct mlx5_vhca_state_event event;
>  };
>  
> +struct mlx5_vhca_event_handler {
> +	struct workqueue_struct *wq;
> +};
> +
> +struct mlx5_vhca_events {
> +	struct mlx5_core_dev *dev;
> +	struct mlx5_vhca_event_handler handler[MLX5_DEV_MAX_WQS];
> +};
> +
>  int mlx5_cmd_query_vhca_state(struct mlx5_core_dev *dev, u16 function_id, u32 *out, u32 outlen)
>  {
>  	u32 in[MLX5_ST_SZ_DW(query_vhca_state_in)] = {};
> @@ -99,6 +108,12 @@ static void mlx5_vhca_state_work_handler(struct work_struct *_work)
>  	kfree(work);
>  }
>  
> +static void
> +mlx5_vhca_events_work_enqueue(struct mlx5_core_dev *dev, int idx, struct work_struct *work)
> +{
> +	queue_work(dev->priv.vhca_events->handler[idx].wq, work);
> +}

I guess you need seprate single-threaded work queues because each
sequence of vhca work items on a given index need to be serialized, but
you don't need to serialize between different vhca index? Makes sense
enough.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>