Message-Id: <e42edd55-2559-437b-a2e3-2773b18d0cc5@app.fastmail.com>
Date: Thu, 31 Jul 2025 02:36:41 +0200
From: "Pierre Barre" <pierre@...re.sh>
To: asmadeus <asmadeus@...ewreck.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc: "Christian Schoenebeck" <linux_oss@...debyte.com>, v9fs@...ts.linux.dev,
 ericvh@...nel.org, lucho@...kov.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] 9p: Use kvmalloc for message buffers

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

Glad that I didn't just create noise. I implemented 9P in addition to NFS in my server because fsyncing on NFS didn't give me what I wanted (fsync doesn't call commit when write is communicated to be "durable" by the server, but that's a story for an other day...). 

I am very grateful that this client implementation exists.

On Thu, Jul 31, 2025, at 00:07, asmadeus@...ewreck.org wrote:
> (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

Powered by blists - more mailing lists