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

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

[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>
---
 drivers/nvme/host/pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index d53550e..fb392c5 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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;
 
-- 
2.7.4

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

[Keith Busch]

On Sun, Jan 14, 2018@08:40:40PM +0900, Minwoo Im wrote:
> 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>

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

[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.

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

[Minwoo Im]

On Mon, Jan 15, 2018@11:08 AM, Keith Busch <keith.busch@intel.com> wrote:
> On Sun, Jan 14, 2018@08:40:40PM +0900, Minwoo Im wrote:
>> 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>

Keith,

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

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

[Minwoo Im]

On Mon, Jan 15, 2018@5:44 PM, Christoph Hellwig <hch@lst.de> wrote:
> 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?