Re: [PATCH net-next v3 01/18] net: Copy slab data for sendmsg(MSG_SPLICE_PAGES)

From
David Howells <dhowells@redhat.com>
Date
2023-06-23 10:01:38
Also in
linux-mm, lkml
Reply to
Paolo Abeni, 2023-06-23
Paolo Abeni [off-list ref] wrote:
quoted
Maybe.  I was trying to put the fast path up at the top without the slow path
bits in it, but I can put the "insufficient_space" bit there.
I *think* you could move the insufficient_space in a separate helped,
that should achieve your goal with fewer labels and hopefully no
additional complexity.
It would require moving things in and out of variables more, but that's
probably fine in the slow path.  The code I derived this from seems to do its
best only to touch memory when it absolutely has to.  But doing what you
suggest would certainly make this more readable, I think.
quoted
quoted
What if fragsz > PAGE_SIZE, we are consistently unable to allocate an
high order page, but order-0, pfmemalloc-ed page allocation is
successful? It looks like this could become an unbounded loop?
It shouldn't.  It should go:

	try_again:
		if (fragsz > offset)
			goto insufficient_space;
	insufficient_space:
		/* See if we can refurbish the current folio. */
		...
I think the critical path is with pfmemalloc-ed pages:

		if (unlikely(cache->pfmemalloc)) {
			__folio_put(folio);
			goto get_new_folio;
		}
I see what you're getting at.  I was thinking that you meant that the critical
bit was that we got into a loop because we never managed to allocate a folio
big enough.

Inserting a check in the event that we fail to allocate an order-3 folio would
take care of that, I think.  After that point, we have a spare folio of
sufficient capacity, even if the folio currently in residence is marked
pfmemalloc.
quoted
quoted
will go through that for every page, even if the expected use-case is
always !PageSlub(page). compound_head() could be costly if the head
page is not hot on cache and I'm not sure if that could be the case for
tcp 0-copy. The bottom line is that I fear a possible regression here.
I can put the PageSlab() check inside the sendpage_ok() so the page flag is
only checked once.  
Perhaps I'm lost again, but AFAICS:

__PAGEFLAG(Slab, slab, PF_NO_TAIL)
...
                PF_POISONED_CHECK(compound_head(page)); })

It looks at compound_head in the end ?!?
Fair point.  Those macros are somewhat hard to read.  Hopefully, all the
compound_head() calls will go away soon-ish.
Thread (39 messages) 39 messages, 6 authors, last reply 2023-06-23
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help