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

[Leon Romanovsky <leon@kernel.org>]

On Thu, Jan 21, 2021 at 07:01:50AM +0000, liweihang wrote:
> On 2021/1/20 16:10, Leon Romanovsky wrote:
> > 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?
> >
>
> Hi Leon,
>
> The current DCA interfaces can be invoked by userspace through ibv_xx_cmd(),
> but it is expected that it can work in ib_post_xx() in kernel in the future.
> Since it may work in context of spin_lock, so we use GFP_ATOMIC.

Are you planning to invoke kzalloc in data path?

The GFP_ATOMIC will cause to use special allocation pool that is seen as precious
resource because it must to succeed.

It is better to avoid this flag if you don't need it.

Thanks