Re: [dpdk-dev] [PATCH] net/mlx5: fix port dead lock when port shutdown

[Yongseok Koh <yskoh@mellanox.com>]

> On May 24, 2018, at 8:20 AM, Xueming Li <xuemingl@mellanox.com> wrote:
> 
> This patch add check to avoid init device MR data twcie which caused
> priv data linked to itself in list.

A typo.

> Fixes: 974f1e7ef146 ("net/mlx5: add new memory region support")
> Cc: yskoh@mellanox.com
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_ethdev.c | 2 ++
> drivers/net/mlx5/mlx5_mr.c     | 1 +
> 2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index c52ec6fb7..7ffd17ff3 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -398,6 +398,8 @@ mlx5_dev_configure(struct rte_eth_dev *dev)
> 	 * deadlock. If it overflows, lookup should be done by searching MR list
> 	 * linearly, which is slow.
> 	 */
> +	if (priv->mr.cache.table)
> +		return 0;

Thanks, nice catch!

Adrien submitted a fix [1] for the similar issue. Adding device to the memory event
callback list is moved to mlx5_pci_probe() as it should be done once. So, how about
moving the rest (mlx5_mr_btree_init()) to mlx5_pci_probe() as well? I mean, instead
of adding this check, you can move the btree_init() above the line where Adrien moved
LIST_INSERT_HEAD() to. You'll need to rebase on Adrien's patch and add a comment to
Shahaf there's dependency.


[1]
net/mlx5: fix crash when configure is not called
http://dpdk.org/dev/patchwork/patch/40404/


Yongseok

> 	if (mlx5_mr_btree_init(&priv->mr.cache, MLX5_MR_BTREE_CACHE_N * 2,
> 			       dev->device->numa_node)) {
> 		/* rte_errno is already set. */
> diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
> index abb1f5179..08105a443 100644
> --- a/drivers/net/mlx5/mlx5_mr.c
> +++ b/drivers/net/mlx5/mlx5_mr.c
> @@ -191,6 +191,7 @@ mlx5_mr_btree_init(struct mlx5_mr_btree *bt, int n, int socket)
> 		rte_errno = EINVAL;
> 		return -rte_errno;
> 	}
> +	assert(!bt->table && !bt->size);
> 	memset(bt, 0, sizeof(*bt));
> 	bt->table = rte_calloc_socket("B-tree table",
> 				      n, sizeof(struct mlx5_mr_cache),
> -- 
> 2.13.3
>