[dpdk-dev] [PATCH] common/mlx5: fix CPU detection for PCI relaxed ordering

Matan Azrad matan at mellanox.com
Sun Jul 19 12:56:03 CEST 2020



From: Thomas Monjalon
> The detection of the CPU was done in a constructor and shared in a global
> variable.
> 
> This variable may not be visible in the net PMD because it was not exported
> as part of the .map file.

Can you explain exactly when it is not visible?

> It is fixed by exporting a function, which is cleaner than a variable.

Can you explain why?
We have classic example - rte_eth_devices.

> By checking the CPU only at the first call of the function, doing the check in a
> constructor becomes useless.

Yes, but why not to do it in constructor? this variable is initialized only once and doesn't depend in any parameter.

> Note: the priority of the constructor was probably irrelevant.
> 
> At the same time, the comments are reworded or dropped if useless.
> 
> Fixes: 4c204fe5e5d2 ("common/mlx5: disable relaxed ordering in unsuitable
> CPUs")
> Cc: shirik at mellanox.com
> Cc: stable at dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
> ---
>  drivers/common/mlx5/linux/mlx5_common_verbs.c |  2 +-
>  drivers/common/mlx5/mlx5_common.c             | 53 ++++++++-----------
>  drivers/common/mlx5/mlx5_common.h             |  4 +-
>  .../common/mlx5/rte_common_mlx5_version.map   |  2 +
>  drivers/net/mlx5/mlx5_flow_dv.c               |  2 +-
>  5 files changed, 28 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/common/mlx5/linux/mlx5_common_verbs.c
> b/drivers/common/mlx5/linux/mlx5_common_verbs.c
> index a2fc7a36bd..31ac20fe09 100644
> --- a/drivers/common/mlx5/linux/mlx5_common_verbs.c
> +++ b/drivers/common/mlx5/linux/mlx5_common_verbs.c
> @@ -55,7 +55,7 @@ mlx5_common_verbs_reg_mr(void *pd, void *addr,
> size_t length,
>  	memset(pmd_mr, 0, sizeof(*pmd_mr));
>  	ibv_mr = mlx5_glue->reg_mr(pd, addr, length,
>  				   IBV_ACCESS_LOCAL_WRITE |
> -				   (haswell_broadwell_cpu ? 0 :
> +				   (mlx5_cpu_is_haswell_broadwell() ? 0 :
>  				   IBV_ACCESS_RELAXED_ORDERING));
>  	if (!ibv_mr)
>  		return -1;
> diff --git a/drivers/common/mlx5/mlx5_common.c
> b/drivers/common/mlx5/mlx5_common.c
> index 693e2c68c8..7232d5131d 100644
> --- a/drivers/common/mlx5/mlx5_common.c
> +++ b/drivers/common/mlx5/mlx5_common.c
> @@ -20,8 +20,6 @@ int mlx5_common_logtype;  const struct mlx5_glue
> *mlx5_glue;  #endif
> 
> -uint8_t haswell_broadwell_cpu;
> -
>  static int
>  mlx5_class_check_handler(__rte_unused const char *key, const char
> *value,
>  			 void *opaque)
> @@ -59,19 +57,8 @@ mlx5_class_get(struct rte_devargs *devargs)  }
> 
> 
> -/* In case this is an x86_64 intel processor to check if
> - * we should use relaxed ordering.
> - */
>  #ifdef RTE_ARCH_X86_64
> -/**
> - * This function returns processor identification and feature information
> - * into the registers.
> - *
> - * @param eax, ebx, ecx, edx
> - *		Pointers to the registers that will hold cpu information.
> - * @param level
> - *		The main category of information returned.
> - */
> +/* Processor identification and feature information filled in
> +registers. */
>  static inline void mlx5_cpu_id(unsigned int level,
>  				unsigned int *eax, unsigned int *ebx,
>  				unsigned int *ecx, unsigned int *edx) @@ -
> 97,17 +84,7 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
>  	mlx5_glue_constructor();
>  }
> 
> -/**
> - * This function is responsible of initializing the variable
> - *  haswell_broadwell_cpu by checking if the cpu is intel
> - *  and reading the data returned from mlx5_cpu_id().
> - *  since haswell and broadwell cpus don't have improved performance
> - *  when using relaxed ordering we want to check the cpu type before
> - *  before deciding whether to enable RO or not.
> - *  if the cpu is haswell or broadwell the variable will be set to 1
> - *  otherwise it will be 0.
> - */
> -RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG)
> +static bool mlx5_x86_is_haswell_broadwell(void)
>  {
>  #ifdef RTE_ARCH_X86_64
>  	unsigned int broadwell_models[4] = {0x3d, 0x47, 0x4F, 0x56}; @@ -
> 125,8 +102,7 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG)
>  	vendor = ebx;
>  	max_level = eax;
>  	if (max_level < 1) {
> -		haswell_broadwell_cpu = 0;
> -		return;
> +		return false;
>  	}
>  	mlx5_cpu_id(1, &eax, &ebx, &ecx, &edx);
>  	model = (eax >> 4) & 0x0f;
> @@ -140,18 +116,31 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu,
> LOG)
>  		if (brand_id == 0 && family == 0x6) {
>  			for (i = 0; i < RTE_DIM(broadwell_models); i++)
>  				if (model == broadwell_models[i]) {
> -					haswell_broadwell_cpu = 1;
> -					return;
> +					return true;
>  				}
>  			for (i = 0; i < RTE_DIM(haswell_models); i++)
>  				if (model == haswell_models[i]) {
> -					haswell_broadwell_cpu = 1;
> -					return;
> +					return true;
>  				}
>  		}
>  	}
>  #endif
> -	haswell_broadwell_cpu = 0;
> +	return false;
> +}
> +
> +/*
> + * Check if the CPU is Intel Haswell or Broadwell,
> + * because PCI relaxed ordering has no performance benefit with these
> CPUs.
> + */
> +bool mlx5_cpu_is_haswell_broadwell(void)
> +{
> +	static bool haswell_broadwell_cpu;
> +	static bool once = false;
> +
> +	if (once)
> +		return haswell_broadwell_cpu;
> +	once = true;
> +	return haswell_broadwell_cpu = mlx5_x86_is_haswell_broadwell();
>  }
> 
>  /**
> diff --git a/drivers/common/mlx5/mlx5_common.h
> b/drivers/common/mlx5/mlx5_common.h
> index 2851507058..d453e0b3d8 100644
> --- a/drivers/common/mlx5/mlx5_common.h
> +++ b/drivers/common/mlx5/mlx5_common.h
> @@ -243,6 +243,9 @@ struct mlx5_klm {
> 
>  LIST_HEAD(mlx5_dbr_page_list, mlx5_devx_dbr_page);
> 
> +__rte_internal
> +bool mlx5_cpu_is_haswell_broadwell(void);
> +
>  __rte_internal
>  enum mlx5_class mlx5_class_get(struct rte_devargs *devargs);
> __rte_internal @@ -255,6 +258,5 @@ int64_t mlx5_get_dbr(void *ctx,  struct
> mlx5_dbr_page_list *head,  __rte_internal  int32_t mlx5_release_dbr(struct
> mlx5_dbr_page_list *head, uint32_t umem_id,
>  			 uint64_t offset);
> -extern uint8_t haswell_broadwell_cpu;
> 
>  #endif /* RTE_PMD_MLX5_COMMON_H_ */
> diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map
> b/drivers/common/mlx5/rte_common_mlx5_version.map
> index ae57ebdba5..501b9fff3b 100644
> --- a/drivers/common/mlx5/rte_common_mlx5_version.map
> +++ b/drivers/common/mlx5/rte_common_mlx5_version.map
> @@ -6,6 +6,8 @@ INTERNAL {
>  	mlx5_common_verbs_reg_mr;
>  	mlx5_common_verbs_dereg_mr;
> 
> +	mlx5_cpu_is_haswell_broadwell;
> +
>  	mlx5_create_mr_ext;
> 
>  	mlx5_dev_to_pci_addr;
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 8b5b6838fa..f1109ae095 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -4201,7 +4201,7 @@ flow_dv_create_counter_stat_mem_mng(struct
> rte_eth_dev *dev, int raws_n)
>  	mkey_attr.klm_num = 0;
>  	if (priv->config.hca_attr.relaxed_ordering_write &&
>  		priv->config.hca_attr.relaxed_ordering_read  &&
> -		!haswell_broadwell_cpu)
> +		!mlx5_cpu_is_haswell_broadwell())
>  		mkey_attr.relaxed_ordering = 1;
>  	mem_mng->dm = mlx5_devx_cmd_mkey_create(sh->ctx,
> &mkey_attr);
>  	if (!mem_mng->dm) {
> --
> 2.27.0



More information about the dev mailing list