linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dominique Martinet <asmadeus@codewreck.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	Christian Schoenebeck <linux_oss@crudebyte.com>,
	v9fs@lists.linux.dev, linux-kernel@vger.kernel.org,
	David Howells <dhowells@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	linux-fsdevel@vger.kernel.org,
	Chris Arges <carges@cloudflare.com>
Subject: Re: [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec
Date: Mon, 15 Dec 2025 06:37:31 -0800	[thread overview]
Message-ID: <aUAdKxcC7195Od5N@infradead.org> (raw)
In-Reply-To: <aT-59HURCGPDUJnZ@codewreck.org>

On Mon, Dec 15, 2025 at 04:34:12PM +0900, Dominique Martinet wrote:
> Christoph Hellwig wrote on Sun, Dec 14, 2025 at 09:55:12PM -0800:
> > > Ok, I don't understand why the current code locks everything down and
> > > wants to use a single scatterlist shared for the whole channel (and
> > > capped to 128 pages?), it should only need to lock around the
> > > virtqueue_add_sg() call, I'll need to play with that some more.
> > 
> > What do you mean with "lock down"?
> 
> Just the odd (to me) use of the chan->lock around basically all of
> p9_virtio_request() and most of p9_virtio_zc_request() -- I'm not pretty
> sure this was just the author trying to avoid an allocation by recycling
> the chan->sg array around though, so ignore this.

Oh, ok.  This seems unrelated to the handling of the iov_iters and
I'm sorry that I don't really know anything about that part.

> 
> > > Looking at other virtio drivers I could probably use a sg_table and
> > > have extract_iter_to_sg() do all the work for us...
> > 
> > Looking at the code I'm actually really confused.  Both because I
> > actually though we were talking about the 9fs direct I/O code, but
> > that has actually been removed / converted to netfs a long time ago.
> >
> > But even more so what the net/9p code is actually doing..  How do
> > we even end up with user addresses here at all?
> 
> FWIW I tried logging and saw ITER_BVEC, ITER_KVEC and ITER_FOLIOQ --
> O_DIRECT writes are seen as BVEC so I guess it's not as direct as I
> expected them to be -- that code could very well be leftovers from
> the switch to iov_iter back in 2015...

Oh right, I think this from Dave's netfs_extract_user_iter.

> (waiting for David's answer here, but as far as I see the contract
> between the transport and the vfs is that the transport should handle
> whatever it's being fed, so it doesn't really matter if it's a bio_vec
> or an iov_iter -- ultimately virtio or whatever backend that wants to
> handle zc likely won't handle bio_vec any better so it'll need
> converting anyway)

Yeah.  Looking at what the code does with the pages, I think all this
should go away in favor of using extract_iter_to_sg and build the
scatterlists directly from the iters, without an extra page indirection.

(and of course one day virtio should migrate away from scatterlists,
but that's for another time).

  parent reply	other threads:[~2025-12-15 14:37 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-09 21:04 [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec Dominique Martinet via B4 Relay
2025-12-10  4:21 ` Matthew Wilcox
2025-12-10  6:04 ` Christoph Hellwig
2025-12-10  7:38   ` asmadeus
2025-12-10  8:32     ` Christoph Hellwig
2025-12-13 13:28       ` asmadeus
2025-12-15  5:55         ` Christoph Hellwig
2025-12-15  7:34           ` Dominique Martinet
2025-12-15 11:16             ` Christian Schoenebeck
2025-12-15 14:37             ` Christoph Hellwig [this message]
2025-12-10 13:33   ` Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aUAdKxcC7195Od5N@infradead.org \
    --to=hch@infradead.org \
    --cc=asmadeus@codewreck.org \
    --cc=carges@cloudflare.com \
    --cc=dhowells@redhat.com \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).