Calculation of Host Memory Buffer sizes

Calculation of Host Memory Buffer sizes

[Thomas Weißschuh]

Hi,

I am wondering about the calculation of the NVMe Host Memory Buffer sizes.
It seems to me that the current algorithm to calculate this size does not lead
to an optimal result.

Hardware information:

mn        : WD_BLACK SN770 1TB
fr        : 731030WD
hmpre     : 51200 (limited by max_host_mem_size_mb to 32768 -> 128MiB)
hmmin     : 823
hmminds   : 0
hmmaxd    : 8

To me this looks like the disk wants 200MiB allocated that can be described in
eight descriptors.
However the kernel log has the following entry:

[    8.981685] nvme nvme0: allocated 32 MiB host memory buffer.

Tracing through drivers/nvme/host/pci.c the following happens:

The loop in nvme_alloc_host_mem() is only entered once.
  min: 3371008
  preferred: 134217728
  min_chunk: 4194304
  chunk_size: 4194304

Now in __nvme_alloc_host_mem() the loop is called the eight times for hmmaxd,
each time allocating 4194304 bytes (4 MiB).
The end result is that a total of 32MiB of Host Memory Buffer are allocated
which is the bare minimum instead of the 200 MiB that are preferred and
available.

It seems that the logic to calculate min_chunk in nvme_alloc_host_mem() starts
with a too small value.

All of this is on a normal x86 laptop with plenty of system memory.
It's reproducible with current git (46cf2c613f4b10eb12f749207b0fd2c1bfae3088)
and 5.17.4.

Thanks,
Thomas

Re: Calculation of Host Memory Buffer sizes

[Thomas Weißschuh]

On 2022-04-28 00:40+0200, Thomas Weißschuh wrote:
> I am wondering about the calculation of the NVMe Host Memory Buffer sizes.
> It seems to me that the current algorithm to calculate this size does not lead
> to an optimal result.
> 
> [..]
> 
> It seems that the logic to calculate min_chunk in nvme_alloc_host_mem() starts
> with a too small value.

Of course shortly after sending the original mail I probably found the solution:

 static int nvme_alloc_host_mem(struct nvme_dev *dev, u64 min, u64 preferred)
 {
-       u64 min_chunk = min_t(u64, preferred, PAGE_SIZE * MAX_ORDER_NR_PAGES);
+       u64 min_chunk = max_t(u64, preferred, PAGE_SIZE * MAX_ORDER_NR_PAGES);

This would also much better fit the comment "start big and work our way down".

I'll send a proper patch tomorrow.

Thomas