Re: [PATCH] nvmet-rdma: use sbitmap to replace rsp free list

[Christoph Hellwig <hch@lst.de>]

On Wed, Sep 25, 2024 at 06:51:00PM +0800, Guixin Liu wrote:
> We can use sbitmap to manage all the nvmet_rdma_rsp instead of using
> free lists and spinlock, and we can use an additional tag to
> determine whether the nvmet_rdma_rsp is extra allocated.

Can you explain why?  I guess it's better performance for some definition
of "performance", but it would be great to state that here, preferably
including numbers.

>  static int nvmet_rdma_alloc_rsp(struct nvmet_rdma_device *ndev,
> -				struct nvmet_rdma_rsp *r);
> +				struct nvmet_rdma_rsp *r,
> +				int tag);

Nit: take can go on the line above.

> -		ret = nvmet_rdma_alloc_rsp(queue->dev, rsp);
> +		ret = nvmet_rdma_alloc_rsp(queue->dev, rsp, -1);

Please add a symbolic name (NVME_RDMA_RSP_DYNAMIC?) for the magic -1
tag.

> @@ -241,17 +236,13 @@ nvmet_rdma_get_rsp(struct nvmet_rdma_queue *queue)
>  static inline void
>  nvmet_rdma_put_rsp(struct nvmet_rdma_rsp *rsp)
>  {
> -	unsigned long flags;
> -
> -	if (unlikely(rsp->allocated)) {
> +	if (unlikely(rsp->tag < 0)) {

.. and use it here.

Otherwise this looks reasonable to me.