Skip to contentSkip to conversation

nvme: pci: replace magic number with readable size macro in hmb code

PATCH New Minwoo Im 3 participants 5 messages Jan 14, 2018 – Jan 17, 2018 NVMe

lore
Share & tools
lore

Minwoo Im

author
#0 lore raw

PATCH 1/2 nvme: pci: replace magic number with readable size macro in hmb code

Those three identify fields(hmpre, hmmin, hmminds) have size in 4KB units.

Replace magic numbers with human-readable size macro.

Signed-off-by Minwoo Im <minwoo.im.dev at gmail.com>
---
 drivers/nvme/host/pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/nvme/host/pci.c +3 -3
index d53550e..fb392c5 100644
@@ -1833,7 +1833,7 @@ static int nvme_alloc_host_mem(struct nvme_dev *dev, u64 min, u64 preferred)
 
 	/* start big and work our way down */
 	for (chunk_size = min_t(u64, preferred, PAGE_SIZE * MAX_ORDER_NR_PAGES);
-	     chunk_size >= max_t(u32, dev->ctrl.hmminds * 4096, PAGE_SIZE * 2);
+	     chunk_size >= max_t(u32, dev->ctrl.hmminds * SZ_4K, PAGE_SIZE * 2);
 	     chunk_size /= 2) {
 		if (!__nvme_alloc_host_mem(dev, preferred, chunk_size)) {
 			if (!min || dev->host_mem_size >= min)

@@ -1848,8 +1848,8 @@ static int nvme_alloc_host_mem(struct nvme_dev *dev, u64 min, u64 preferred)
 static int nvme_setup_host_mem(struct nvme_dev *dev)
 {
 	u64 max = (u64)max_host_mem_size_mb * SZ_1M;
-	u64 preferred = (u64)dev->ctrl.hmpre * 4096;
-	u64 min = (u64)dev->ctrl.hmmin * 4096;
+	u64 preferred = (u64)dev->ctrl.hmpre * SZ_4K;
+	u64 min = (u64)dev->ctrl.hmmin * SZ_4K;
 	u32 enable_bits = NVME_HOST_MEM_ENABLE;
 	int ret = 0;
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/2 nvme: pci: replace magic number with readable size macro in hmb code

Quoted 6 lines · Minwoo Im

Those three identify fields(hmpre, hmmin, hmminds) have size in 4KB units.

Replace magic numbers with human-readable size macro.

Signed-off-by Minwoo Im <minwoo.im.dev at gmail.com>

Looks fine, though I thought an explicit #include <linux/sizes.h> is in order since it's not obvious how we're pulling in these defines, but there's already other SZ_* macro uses in this file, so it must be safe!

Reviewed-by Keith Busch <keith.busch at intel.com>
author
#2 lore raw

PATCH 1/2 nvme: pci: replace magic number with readable size macro in hmb code

Quoted 6 lines · Keith Busch
Quoted 5 lines · Minwoo Im

Those three identify fields(hmpre, hmmin, hmminds) have size in 4KB units.

Replace magic numbers with human-readable size macro.

Quoted 7 lines
Quoted 1 line
Signed-off-by Minwoo Im <minwoo.im.dev at gmail.com>

Looks fine, though I thought an explicit #include <linux/sizes.h> is in order since it's not obvious how we're pulling in these defines, but there's already other SZ_* macro uses in this file, so it must be safe!

Reviewed-by Keith Busch <keith.busch at intel.com>

Keith,

Thanks for your kind review.
I agree that explicit #include always seems right.

maintainer
#3 lore raw

PATCH 1/2 nvme: pci: replace magic number with readable size macro in hmb code

In reply to Minwoo Im

SZ_4K does not add any value at all. The SZ constants may make some size in the megabyte and larger ranges where the multiples of 1024 aren't totally obviously, but they certainly don't for 4 digit values.

author
#4 lore raw

PATCH 1/2 nvme: pci: replace magic number with readable size macro in hmb code

Quoted 3 lines · Christoph Hellwig

SZ_4K does not add any value at all. The SZ constants may make some size in the megabyte and larger ranges where the multiples of 1024 aren't totally obviously, but they certainly don't for 4 digit values.

Christoph,

Thanks for giving your kind review.
Do you mean that SZ_* constants should _not_ be used to represent small size like 4K?