Re: [PATCH RFC 1/7] RDMA/hns: Introduce DCA for RC QP

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jan 15, 2021 at 06:22:12PM +0800, Weihang Li wrote:
> From: Xi Wang <wangxi11@huawei.com>
>
> The hip09 introduces the DCA(Dynamic context attachment) feature which
> supports many RC QPs to share the WQE buffer in a memory pool, this will
> reduce the memory consumption when there are too many QPs are inactive.
>
> If a QP enables DCA feature, the WQE's buffer will not be allocated when
> creating. But when the users start to post WRs, the hns driver will
> allocate a buffer from the memory pool and then fill WQEs which tagged with
> this QP's number.
>
> The hns ROCEE will stop accessing the WQE buffer when the user polled all
> of the CQEs for a DCA QP, then the driver will recycle this WQE's buffer
> to the memory pool.
>
> This patch adds a group of methods to support the user space register
> buffers to a memory pool which belongs to the user context. The hns kernel
> driver will update the pages state in this pool when the user calling the
> post/poll methods and the user driver can get the QP's WQE buffer address
> by the key and offset which queried from kernel.
>
> Signed-off-by: Xi Wang <wangxi11@huawei.com>
> Signed-off-by: Weihang Li <liweihang@huawei.com>
> ---
>  drivers/infiniband/hw/hns/Makefile          |   2 +-
>  drivers/infiniband/hw/hns/hns_roce_dca.c    | 381 ++++++++++++++++++++++++++++
>  drivers/infiniband/hw/hns/hns_roce_dca.h    |  22 ++
>  drivers/infiniband/hw/hns/hns_roce_device.h |  10 +
>  drivers/infiniband/hw/hns/hns_roce_main.c   |  27 +-
>  include/uapi/rdma/hns-abi.h                 |  23 ++
>  6 files changed, 462 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_dca.c
>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_dca.h

<...>

> +static struct dca_mem *alloc_dca_mem(struct hns_roce_dca_ctx *ctx)
> +{
> +	struct dca_mem *mem, *tmp, *found = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ctx->pool_lock, flags);
> +	list_for_each_entry_safe(mem, tmp, &ctx->pool, list) {
> +		spin_lock(&mem->lock);
> +		if (dca_mem_is_free(mem)) {
> +			found = mem;
> +			set_dca_mem_alloced(mem);
> +			spin_unlock(&mem->lock);
> +			goto done;
> +		}
> +		spin_unlock(&mem->lock);
> +	}
> +
> +done:
> +	spin_unlock_irqrestore(&ctx->pool_lock, flags);
> +
> +	if (found)
> +		return found;
> +
> +	mem = kzalloc(sizeof(*mem), GFP_ATOMIC);

Should it be ATOMIC?

> +	if (!mem)
> +		return NULL;
> +
> +	spin_lock_init(&mem->lock);
> +	INIT_LIST_HEAD(&mem->list);
> +
> +	set_dca_mem_alloced(mem);
> +
> +	spin_lock_irqsave(&ctx->pool_lock, flags);
> +	list_add(&mem->list, &ctx->pool);
> +	spin_unlock_irqrestore(&ctx->pool_lock, flags);
> +	return mem;
> +}

<...>

>  /**
>   * hns_get_gid_index - Get gid index.
> @@ -306,15 +308,16 @@ static int hns_roce_modify_device(struct ib_device *ib_dev, int mask,
>  static int hns_roce_alloc_ucontext(struct ib_ucontext *uctx,
>  				   struct ib_udata *udata)
>  {
> -	int ret;
>  	struct hns_roce_ucontext *context = to_hr_ucontext(uctx);
> -	struct hns_roce_ib_alloc_ucontext_resp resp = {};
>  	struct hns_roce_dev *hr_dev = to_hr_dev(uctx->device);
> +	struct hns_roce_ib_alloc_ucontext_resp resp = {};
> +	int ret;
>
>  	if (!hr_dev->active)
>  		return -EAGAIN;
>
>  	resp.qp_tab_size = hr_dev->caps.num_qps;
> +	resp.cap_flags = (u32)hr_dev->caps.flags;

This is prone to errors, flags is u64.

<...>

> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> index 90b739d..f59abc4 100644
> --- a/include/uapi/rdma/hns-abi.h
> +++ b/include/uapi/rdma/hns-abi.h
> @@ -86,10 +86,33 @@ struct hns_roce_ib_create_qp_resp {
>  struct hns_roce_ib_alloc_ucontext_resp {
>  	__u32	qp_tab_size;
>  	__u32	cqe_size;
> +	__u32	cap_flags;
>  };

This struct should be padded to 64bits,

Thanks