On Thu, Feb 21, 2019 at 9:22 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
>
> On Thu, Feb 21, 2019 at 10:17:28AM -0800, Matthew Wilcox wrote:
> > diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> > index 90f1b0bae5b5..3326e07ab7ae 100644
> > +++ b/drivers/infiniband/hw/mlx5/cq.c
> > @@ -522,7 +522,7 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
> > case MLX5_CQE_SIG_ERR:
> > sig_err_cqe = (struct mlx5_sig_err_cqe *)cqe64;
> >
> > - read_lock(&dev->mdev->priv.mkey_table.lock);
> > + xa_lock(&dev->mdev->priv.mkey_table);
> > mmkey = __mlx5_mr_lookup(dev->mdev,
> > mlx5_base_mkey(be32_to_cpu(sig_err_cqe->mkey)));
> > mr = to_mibmr(mmkey);
> > @@ -537,7 +537,7 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
> > mr->sig->err_item.expected,
> > mr->sig->err_item.actual);
> >
> > - read_unlock(&dev->mdev->priv.mkey_table.lock);
> > + xa_unlock(&dev->mdev->priv.mkey_table);
> > goto repoll;
> > }
>
> I think this xa_lock() should really be
>
> srcu_read_lock(&dev->mr_srcu)
>
> Moni?
>
When mkey table was implemented as radix_tree I believe that
read_unlock needed to be replaces with srcu_read_lock() (like in all
other places that lookup mkeys).
When mkey table is implemented as xarray I'm not sure. It's not clear
to me from the documentation what are the requirements.
> > diff --git a/include/linux/mlx5/qp.h b/include/linux/mlx5/qp.h
> > index b26ea9077384..5b3fb2642d0c 100644
> > +++ b/include/linux/mlx5/qp.h
> > @@ -552,7 +552,7 @@ static inline struct mlx5_core_qp *__mlx5_qp_lookup(struct mlx5_core_dev *dev, u
> >
> > static inline struct mlx5_core_mkey *__mlx5_mr_lookup(struct mlx5_core_dev *dev, u32 key)
> > {
> > - return radix_tree_lookup(&dev->priv.mkey_table.tree, key);
> > + return xa_load(&dev->priv.mkey_table, key);
>
> I think this actually fixes a bug, as pagefault_single_data_segment is
> calling radix_tree_lookup only under srcu, which is not compatible
> with the radix tree's use of kfree_rcu ?
>
> Jason
