[PATCH 6/7] nvme-pci: add a symolic name for the small pool size

[Caleb Sander Mateos]

On Tue, May 13, 2025 at 12:00 AM Christoph Hellwig <hch at lst.de> wrote:
>
> From: Leon Romanovsky <leon at kernel.org>
>
> Open coding magic numbers in multiple places is never a good idea.
>
> Signed-off-by: Leon Romanovsky <leon at kernel.org>
> [hch: split from a larger patch]
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>  drivers/nvme/host/pci.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 16e2ce25da83..5396fe30eb94 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -37,6 +37,8 @@
>
>  #define SGES_PER_PAGE  (NVME_CTRL_PAGE_SIZE / sizeof(struct nvme_sgl_desc))
>
> +#define NVME_SMALL_POOL_SIZE   256
> +
>  /*
>   * These can be higher, but we need to ensure that any command doesn't
>   * require an sg allocation that needs more than a page of data.
> @@ -407,7 +409,7 @@ static struct nvme_descriptor_pools *
>  nvme_setup_descriptor_pools(struct nvme_dev *dev, unsigned numa_node)
>  {
>         struct nvme_descriptor_pools *pools = &dev->descriptor_pools[numa_node];
> -       size_t small_align = 256;
> +       size_t small_align = NVME_SMALL_POOL_SIZE;
>
>         if (pools->small)
>                 return pools; /* already initialized */
> @@ -422,7 +424,7 @@ nvme_setup_descriptor_pools(struct nvme_dev *dev, unsigned numa_node)
>
>         /* Optimisation for I/Os between 4k and 128k */

This comment explains the choice of 256. Maybe it would make sense to
move it to the definition of NVME_SMALL_POOL_SIZE?

>         pools->small = dma_pool_create_node("nvme descriptor 256", dev->dev,

The magic number is used in this string too. Maybe change "256" to "small"?

Other than that,
Reviewed-by: Caleb Sander Mateos <csander at purestorage.com>


> -                       256, small_align, 0, numa_node);
> +                       NVME_SMALL_POOL_SIZE, small_align, 0, numa_node);
>         if (!pools->small) {
>                 dma_pool_destroy(pools->large);
>                 pools->large = NULL;
> @@ -689,7 +691,7 @@ static blk_status_t nvme_pci_setup_prps(struct nvme_queue *nvmeq,
>         }
>
>         if (DIV_ROUND_UP(length, NVME_CTRL_PAGE_SIZE) <=
> -           256 / sizeof(__le64))
> +           NVME_SMALL_POOL_SIZE / sizeof(__le64))
>                 iod->flags |= IOD_SMALL_DESCRIPTOR;
>
>         prp_list = dma_pool_alloc(nvme_dma_pool(nvmeq, iod), GFP_ATOMIC,
> @@ -774,7 +776,7 @@ static blk_status_t nvme_pci_setup_sgls(struct nvme_queue *nvmeq,
>                 return BLK_STS_OK;
>         }
>
> -       if (entries <= 256 / sizeof(*sg_list))
> +       if (entries <= NVME_SMALL_POOL_SIZE / sizeof(*sg_list))
>                 iod->flags |= IOD_SMALL_DESCRIPTOR;
>
>         sg_list = dma_pool_alloc(nvme_dma_pool(nvmeq, iod), GFP_ATOMIC,
> --
> 2.47.2
>