[PATCH RFC] nvme-pci: remove the unsed code in alloc_size

[PATCH RFC] nvme-pci: remove the unsed code in alloc_size

[Chaitanya Kulkarni]

From the initial implementation of NVMe SGL kernel support
commit a7a7cbe353a5 ("nvme-pci: add SGL support") with addition of the
commit 943e942e6266 ("nvme-pci: limit max IO size and segments to avoid
high order allocations") now there is only caller left for
nvme_pci_iod_alloc_size() which statically passes true for last
parameter that calculates allocation size based on SGL since we need
size of biggest command supported for mempool allocation.

This patch removes the helper functions nvme_pci_iod_alloc_size() and
nvme_npages() used for allocation size calculation and open codes it
into nvme_probe().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
Hi,

We can get rid of unused code with this patch, would like to get some
feedback, Tagging it as an RFC since I've only tested fio verification
and did not test all the cases with sgl and prp.

If we decided to get it I can put more effort on this.

Regards,
Chaitanya
---
 drivers/nvme/host/pci.c | 30 +++---------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 07ac28d7d66c..aad8cc176219 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -341,18 +341,6 @@ static bool nvme_dbbuf_update_and_check_event(u16 value, u32 *dbbuf_db,
 	return true;
 }
 
-/*
- * Will slightly overestimate the number of pages needed.  This is OK
- * as it only leads to a small amount of wasted memory for the lifetime of
- * the I/O.
- */
-static int nvme_npages(unsigned size, struct nvme_dev *dev)
-{
-	unsigned nprps = DIV_ROUND_UP(size + dev->ctrl.page_size,
-				      dev->ctrl.page_size);
-	return DIV_ROUND_UP(8 * nprps, PAGE_SIZE - 8);
-}
-
 /*
  * Calculates the number of pages needed for the SGL segments. For example a 4k
  * page can accommodate 256 SGL descriptors.
@@ -362,19 +350,6 @@ static int nvme_pci_npages_sgl(unsigned int num_seg)
 	return DIV_ROUND_UP(num_seg * sizeof(struct nvme_sgl_desc), PAGE_SIZE);
 }
 
-static unsigned int nvme_pci_iod_alloc_size(struct nvme_dev *dev,
-		unsigned int size, unsigned int nseg, bool use_sgl)
-{
-	size_t alloc_size;
-
-	if (use_sgl)
-		alloc_size = sizeof(__le64 *) * nvme_pci_npages_sgl(nseg);
-	else
-		alloc_size = sizeof(__le64 *) * nvme_npages(size, dev);
-
-	return alloc_size + sizeof(struct scatterlist) * nseg;
-}
-
 static int nvme_admin_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
 				unsigned int hctx_idx)
 {
@@ -2814,8 +2789,9 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	 * Double check that our mempool alloc size will cover the biggest
 	 * command we support.
 	 */
-	alloc_size = nvme_pci_iod_alloc_size(dev, NVME_MAX_KB_SZ,
-						NVME_MAX_SEGS, true);
+	alloc_size = sizeof(__le64 *) * nvme_pci_npages_sgl(NVME_MAX_SEGS) +
+		sizeof(struct scatterlist) * NVME_MAX_SEGS;
+
 	WARN_ON_ONCE(alloc_size > PAGE_SIZE);
 
 	dev->iod_mempool = mempool_create_node(1, mempool_kmalloc,
-- 
2.22.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH RFC] nvme-pci: remove the unsed code in alloc_size

[Christoph Hellwig]

On Mon, Jul 06, 2020 at 03:53:05PM -0700, Chaitanya Kulkarni wrote:
> >From the initial implementation of NVMe SGL kernel support
> commit a7a7cbe353a5 ("nvme-pci: add SGL support") with addition of the
> commit 943e942e6266 ("nvme-pci: limit max IO size and segments to avoid
> high order allocations") now there is only caller left for
> nvme_pci_iod_alloc_size() which statically passes true for last
> parameter that calculates allocation size based on SGL since we need
> size of biggest command supported for mempool allocation.
> 
> This patch removes the helper functions nvme_pci_iod_alloc_size() and
> nvme_npages() used for allocation size calculation and open codes it
> into nvme_probe().

This is an obviously correct transformation, but I think the existing
code is a land mine.  I think we should change it to use the maximum
of either the PRP or SGL case to gurantee that we are on the safe
side.  Can you do that instead?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH RFC] nvme-pci: remove the unsed code in alloc_size

[Chaitanya Kulkarni]

On 7/7/20 01:47, Christoph Hellwig wrote:
> On Mon, Jul 06, 2020 at 03:53:05PM -0700, Chaitanya Kulkarni wrote:
>> >From the initial implementation of NVMe SGL kernel support
>> commit a7a7cbe353a5 ("nvme-pci: add SGL support") with addition of the
>> commit 943e942e6266 ("nvme-pci: limit max IO size and segments to avoid
>> high order allocations") now there is only caller left for
>> nvme_pci_iod_alloc_size() which statically passes true for last
>> parameter that calculates allocation size based on SGL since we need
>> size of biggest command supported for mempool allocation.
>>
>> This patch removes the helper functions nvme_pci_iod_alloc_size() and
>> nvme_npages() used for allocation size calculation and open codes it
>> into nvme_probe().
> This is an obviously correct transformation, but I think the existing
> code is a land mine.  I think we should change it to use the maximum
> of either the PRP or SGL case to gurantee that we are on the safe
> side.  Can you do that instead?
> 

Okay let me see I'll send out a patch with your suggestion.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme