Re: [PATCH] nvme-fc: Improve memory usage in nvme_fc_rcv_ls_req()

[James Smart <jsmart2021@gmail.com>]

On 10/2/2022 2:59 AM, Christophe JAILLET wrote:
> sizeof( struct nvmefc_ls_rcv_op ) = 64
> sizeof( union nvmefc_ls_requests ) = 1024
> sizeof( union nvmefc_ls_responses ) = 128
> 
> So, in nvme_fc_rcv_ls_req(), 1216 bytes of memory are requested when
> kzalloc() is called.
> 
> Because of the way memory allocations are performed, 2048 bytes are
> allocated. So about 800 bytes are wasted for each request.
> 
> Switch to 3 distinct memory allocations, in order to:
>     - save these 800 bytes
>     - avoid zeroing this extra memory
>     - make sure that memory is properly aligned in case of DMA access
>      ("fc_dma_map_single(lsop->rspbuf)" just a few lines below)
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is only a RFC to see if this kind of approach makes sense or
> not.
> I've not checked all paths, so it is likely that it is incomplete.

I think it's ok.  some of the other paths that behave similarly may be 
aware of the contiguous allocation, but I don't think this one is.

> 
> Anyway, it is just a trade-of between memory footprint and CPU usage (3
> kzalloc() instead of 1)
> 
> I don't know if it is a slow path or not, nor if the "rport->ls_rcv_list"
> list can get big (each item overuses these 800 bytes of memory)

It is slow path/not often and the ls typically doesn't stay outstanding 
long. thus it hasn't been critical to optimize.

Should be few items on ls_rcv_list, but in rare conditions there could 
be a burst.

> 
> 3 kzalloc is more than just 1 (sic!), but with this patch, 800 bytes are
> not zeroed anymore. Moreover, maybe the zeroing of rqstbuf and/or rspbuf
> can be saved as well.
> So, it could balance the impact of the 3 kzalloc().

zeroing of rqstbuf may be skipped, as the meaningful part of the buffer 
will be memcpy'd a few lines below.

rspbuf should be zero'd.


> 
> So, if it looks promising, s.o. with the corresponding hardware should
> make some measurements on memory and CPU usage.
> ---
>   drivers/nvme/host/fc.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 

I think the mods are fine

Reviewed-by: James Smart <jsmart2021@gmail.com>

-- james