Re: [PATCH 1/2] nvme-pci: fix freeing single sgl

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Fri, 2023-02-10 at 10:03 -0800, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> There may only be a single DMA mapped entry from multiple physical
> segments, which means we don't allocate a separte SGL list. Check the
> number of allocations prior to know if we need to free something.
> 
> Freeing a single list allocation is the same for both PRP and SGL
> usages, so we don't need to check the use_sgl flag anymore.
> 
> Fixes: 01df742d8c5c0 ("nvme-pci: remove SGL segment descriptors")
> Reported-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> Niklas,
> This is a little different than the one you tested, so I didn't include
> your "Tested-by". I'm confident this is patch still fixes the issue,
> though.
> 
>  drivers/nvme/host/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index a331fbfa9a667..47d6b0023e3a8 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -556,7 +556,7 @@ static void nvme_unmap_data(struct nvme_dev *dev, struct request *req)
>  	if (iod->nr_allocations == 0)
>  		dma_pool_free(dev->prp_small_pool, iod->list[0].sg_list,
>  			      iod->first_dma);
> -	else if (iod->use_sgl)
> +	else if (iod->nr_allocations == 1)
>  		dma_pool_free(dev->prp_page_pool, iod->list[0].sg_list,
>  			      iod->first_dma);
>  	else


Ah nice that makes it look cleaner and you got rid of the use_sgl flag.
I gave the two new patches a quick test on top of today's linux-next
and they work fine too. In fact while I don't have hard numbers and my
environment is quite noisy and there is of course other changes at play
too but linux-next does seem to get a bit more IOPS than v6.2-rc8 in my
FIO 4k random read test.

Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>