Re: Reorganising how the networking layer handles memory

[Christoph Hellwig <hch@infradead.org>]

On Tue, May 06, 2025 at 02:50:49PM +0100, David Howells wrote:
> > > (2) sendmsg(MSG_ZEROCOPY) suffers from the O_DIRECT vs fork() bug because
> > >      it doesn't use page pinning.  It needs to use the GUP routines.
> > 
> > We end up calling iov_iter_get_pages2(). Is it not setting
> > FOLL_PIN is a conscious choice, or nobody cared until now?
> 
> iov_iter_get_pages*() predates GUP, I think.

It predates pin_user_pages, but get_user_pages is much older.

> There's now an
> iov_iter_extract_pages() that does the pinning stuff, but you have to do a
> different cleanup, which is why I created a new API call.

But yes, iov_iter_get_pages* needs to go away in favour of
iov_iter_extract_pages, and I'm still annoyed that despite multiple
pings no one has done any work on that outside of block / block based
direct I/O and netfs.

> > >  (3) sendmsg(MSG_SPLICE_PAGES) isn't entirely satisfactory because it can't be
> > >      used with certain memory types (e.g. slab).  It takes a ref on whatever
> > >      it is given - which is wrong if it should pin this instead.
> > 
> > s/takes a ref/requires a ref/ ? I mean - the caller implicitly grants 
> > a ref  to the stack, right? But yes, the networking stack will try to
> > release it.
> 
> I mean 'takes' as in skb_append_pagefrags() calls get_page() - something that
> needs to be changed.
> 
> Christoph Hellwig would like to make it such that the extractor gets
> {phyaddr,len} rather than {page,off,len} - so all you, the network layer, see
> is that you've got a span of memory to use as your buffer.  How that span of
> memory is managed is the responsibility of whoever called sendmsg() - and they
> need a callback to be able to handle that.

Not sure what the extractor is, but we plan to change the bio_vec
to be physical address instead of page+offset based.  Where we is
a lot more people than just me.

> Once advantage of delegating it to the caller, though, and having the caller
> keep track of which bits in still needs to hold on to by transmission
> completion position is that we don't need to manage refs/pins across sk_buff
> duplication - let alone what we should do with stuff that's kmalloc'd.

And the callers already do that for all other kinds of I/O anyway.