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

[liweihang <liweihang@huawei.com>]

On 2021/1/21 16:53, Leon Romanovsky wrote:
> 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

We need to allocate memory while spin_lock is hold, how about using GFP_KERNEL or
GFP_NOWAIT?

Thanks
Weihang