Re: [PATCH] 9p: Use kvmalloc for message buffers

[asmadeus@codewreck.org]

(Added Willy in To, if you have time to advise on what's appropriate
wrt. memory allocation here to use either as a contiguous virtual memory
buffer or a scatterlist for various kind of dma used by transports
that'd be appreciated)


First - thanks for starting this thread, large contiguous allocations
has been a problem reported regularly in the past, and no-one has had
the energy to address it yet, but it's definitely something worth
tackling.


Pierre Barre wrote on Wed, Jul 30, 2025 at 10:16:13PM +0200:
> As a middle-ground, would it be acceptable to add a transport
> capability flag indicating whether the transport requires contiguous
> memory for DMA?

I think such a flag will be needed even if we do what Christian suggested:
>> So a more reasonable solution instead might be using kvmalloc(), as suggested
>> by you, and adjusting the individual transports such that they translate a
>> virtual memory address to a list of physical addresses via e.g.
>> vmalloc_to_page() if needed.

Start with a requires contiguous flag set for all transports except
trans_fd (I'm not sure about usb, but I'm pretty sure all the rest need
it); then each transport can figure out how to get a scatterlist or
something they need from the allocation, so we don't need to have a big
bang that breaks everything at the same time.

I'm sure virtio's implem will come soon enough, but I don't see anyone
working on RDMA or xen so fast.

> 1. Add a P9_TRANS_REQUIRES_CONTIGUOUS flag to struct p9_trans_module
> 2. Set this flag for virtio, xen, and rdma transports

This is a nit but I'd rather the flag go the other way around, e.g. no
flag means requires contiguous and it's only set after confirming the
transport works
(I just looked at usb and struct usb_request makes me think some drivers
use dma, so, yeah..)

We can always turn it around later if the majority of transports handle
it.


> 3. Modify p9_fcall_init to check the flag:
>      if (c->trans_mod->caps & P9_TRANS_REQUIRES_CONTIGUOUS)
>         fc->sdata = kmalloc(alloc_msize, GFP_NOFS);
>      else
>         fc->sdata = kvmalloc(alloc_msize, GFP_NOFS);

I'm also curious if there's something more appropriate than kvmalloc wrt
using either as a contiguous virtual memory buffer or a scatterlist, but
I think this is fine for now unless someone knows of something more
appropriate.


Thanks,
-- 
Dominique Martinet | Asmadeus