Re: [PATCH v3 8/9] IB/mlx5: Add helper mlx5_ib_post_send_wait

Leon Romanovsky
2016-10-27T06:09:58Z

[-- Attachment #1: Type: text/plain, Size: 3204 bytes --]

On Wed, Oct 26, 2016 at 01:31:13PM +0530, Binoy Jayan wrote:
> Clean up the following common code (to post a list of work requests to the
> send queue of the specified QP) at various places and add a helper function
> 'mlx5_ib_post_send_wait' to implement the same.
>
> - Initialize 'mlx5_ib_umr_context' on stack
> - Assign "mlx5_umr_wr:wr:wr_cqe to umr_context.cqe
> - Acquire the semaphore
> - call ib_post_send with a single ib_send_wr
> - wait_for_completion()
> - Check for umr_context.status
> - Release the semaphore
>
> As semaphores are going away in the future, moving all of these into the
> shared helper leaves only a single function using the semaphore, which
> can then be rewritten to use something else.
>
> Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/infiniband/hw/mlx5/mr.c | 115 +++++++++++-----------------------------
> 1 file changed, 32 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index d4ad672..19c292a 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -856,16 +856,40 @@ static inline void mlx5_ib_init_umr_context(struct mlx5_ib_umr_context *context)
> init_completion(&context->done);
> }
>
> +static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
> + struct mlx5_umr_wr *umrwr)
> +{
> + struct umr_common *umrc = &dev->umrc;
> + struct ib_send_wr *bad;
> + int err;
> + struct mlx5_ib_umr_context umr_context;
> +
> + mlx5_ib_init_umr_context(&umr_context);
> + umrwr->wr.wr_cqe = &umr_context.cqe;
> +
> + down(&umrc->sem);
> + err = ib_post_send(umrc->qp, &umrwr->wr, &bad);
> + if (err) {
> + mlx5_ib_warn(dev, "UMR post send failed, err %d\n", err);
> + } else {
> + wait_for_completion(&umr_context.done);
> + if (umr_context.status != IB_WC_SUCCESS) {
> + mlx5_ib_warn(dev, "reg umr failed (%u)\n",
> + umr_context.status);
> + err = -EFAULT;
> + }
> + }
> + up(&umrc->sem);
> + return err;
> +}
> +
> static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
> u64 virt_addr, u64 len, int npages,
> int page_shift, int order, int access_flags)
> {
> struct mlx5_ib_dev *dev = to_mdev(pd->device);
> struct device *ddev = dev->ib_dev.dma_device;
> - struct umr_common *umrc = &dev->umrc;
> - struct mlx5_ib_umr_context umr_context;
> struct mlx5_umr_wr umrwr = {};
> - struct ib_send_wr *bad;
> struct mlx5_ib_mr *mr;
> struct ib_sge sg;
> int size;
> @@ -894,24 +918,12 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
> if (err)
> goto free_mr;
>
> - mlx5_ib_init_umr_context(&umr_context);
> -
> - umrwr.wr.wr_cqe = &umr_context.cqe;
> prep_umr_reg_wqe(pd, &umrwr.wr, &sg, dma, npages, mr->mmkey.key,
> page_shift, virt_addr, len, access_flags);
>
> - down(&umrc->sem);
> - err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
> - if (err) {
> - mlx5_ib_warn(dev, "post send failed, err %d\n", err);
> + err = mlx5_ib_post_send_wait(dev, &umrwr);
> + if (err != -EFAULT)
> goto unmap_dma;

NAK,
You are breaking driver.

it should be "if (err && err != -EFAULT)"

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]