All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Li <hao.li@linux.dev>
To: Harry Yoo <harry@kernel.org>
Cc: vbabka@kernel.org, akpm@linux-foundation.org, cl@gentwo.org,
	 rientjes@google.com, roman.gushchin@linux.dev,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/slub: refill prefilled sheaves from the barn
Date: Mon, 21 Sep 2026 12:21:14 +0800	[thread overview]
Message-ID: <arCeRr44nni8hykr@fedora> (raw)
In-Reply-To: <aq0ylDidEHa2kg4X@thinkstation>

On Fri, Sep 18, 2026 at 04:35:15PM +0100, Harry Yoo wrote:
> On Fri, Sep 18, 2026 at 07:41:56PM +0800, Hao Li wrote:
> > Currently, when the prefill API refills a non-full sheaf, it takes the
> > objects from partial slabs and never from the full sheaves in the barn,
> > so once the barn's full list becomes saturated, it stays saturated.
> > 
> > For objects freed via kfree_rcu(), every RCU sheaf then has to be
> > flushed to slabs because the barn's full list has no room.
> > 
> > To fix this, let the sheaf refill from the barn first, and introduce a
> > partial sheaf in the barn, which holds the leftover objects.
> > 
> > If no partial sheaf exists, take a full sheaf from the barn and return
> > it to the caller; the non-full sheaf becomes the partial sheaf, or is
> > put on the barn's empty list if it holds no objects.
> > 
> > If a partial sheaf exists and the non-full sheaf and the partial sheaf
> > together reach capacity, the one holding more objects is filled up from
> > the other and handed out instead, so no full sheaf needs to be taken
> > from the barn.
> > 
> > If a partial sheaf exists but the two together do not reach capacity,
> > take a full sheaf from the barn for the caller; the objects of the
> > non-full sheaf are absorbed into the partial sheaf, and the non-full
> > sheaf, now empty, is put on the barn's empty list.
> 
> Perhaps let's explain why the slab allocator needs this partial sheaf
> handling only for prefilled sheaves path?

Agreed, that's definitely worth an explanation here!

> 
> while reviewing it i was wondering "why is this just not part of
> refill_sheaf()"
> 
> I assume the reason is: usually we only refill empty sheaves because
> if it has at least one object, it can serve the allocation.

I think the distinction is less about why refill_sheaf() doesn't use
sheaf_partial, and more about what the two paths do beforehand.

For simplicity, let's call the __pcs_replace_empty_main path the
generic path, to distinguish it from the prefill path.

In both paths, refill_sheaf() plays an equivalent role: it simply pulls objects
from n->partial. Since neither involves the barn at that point, sheaf_partial
doesn't really come into play there.

The real difference is what happens before calling refill_sheaf(). The generic
path cleanly swaps an empty sheaf for a full one from the barn. But on the
prefill path, the incoming sheaf isn't necessarily empty, so taking objects
from the barn usually leaves leftovers. That's why we need a data structure
to temporarily hold those leftovers, and that's where sheaf_partial comes in.

If this explanation makes sense, I'll add it to the commit message. :)

> 
> > the gain comes from two sides: every full sheaf taken out makes room on
> > the barn's full list for a future rcu sheaf, and refilling from the
> > barn is cheaper than refilling from partial slabs under list_lock.
> > 
> > note that putting the non-full sheaf on the full list instead would not
> > work: it would occupy room on the full list, so the list would stay
> > saturated and rcu_free_sheaf() would still keep flushing.
> 
> [..]
> 
> > ---
> >  mm/slub.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 134 insertions(+), 8 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 54ec12503357..0b1de6b42b61 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3164,6 +3165,89 @@ static struct slab_sheaf *barn_get_empty_sheaf(struct node_barn *barn,
> >  	return empty;
> >  }
> >  
> > +/*
> > + * Exchange @sheaf, which holds fewer objects than requested, for a full one,
> > + * keeping the leftover objects in the barn's partial sheaf instead of
> > + * flushing them.
> > + *
> > + * Returns a full sheaf, or NULL if the barn cannot make one.
> > + * The returned sheaf might be @sheaf itself or a new one.
> > + */
> > +static struct slab_sheaf *barn_replace_partial_sheaf(struct kmem_cache *s,
> > +						     struct node_barn *barn,
> > +						     struct slab_sheaf *sheaf)
> > +{
> > +	struct slab_sheaf *full = NULL, *partial;
> > +	unsigned int to_move;
> > +	unsigned long flags;
> > +
> > +	if (!data_race(barn->nr_full) && !data_race(barn->sheaf_partial))
> > +		return NULL;
> > +
> > +	spin_lock_irqsave(&barn->lock, flags);
> > +
> > +	partial = barn->sheaf_partial;
> > +	if (partial && partial->size + sheaf->size >= s->sheaf_capacity) {
> > +		/* Fill the larger one to capacity from the smaller */
> > +		if (partial->size > sheaf->size)
> > +			swap(partial, sheaf);
> 
> Hmm but why switch sheaves when we don't have to?

My thinking was to minimize the amount of data copied via memcpy as much as
possible, since I was a bit concerned that the copy overhead might stretch the
lock hold time in the critical section.

> Sounds like we're losing cache affinity unnecessarily.

Yeah, that makes sense to me.

I'll reply to the other points in the next email.

-- 
Thanks,
Hao


      parent reply	other threads:[~2026-09-21  4:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 11:41 [PATCH] mm/slub: refill prefilled sheaves from the barn Hao Li
2026-09-18 15:35 ` Harry Yoo
2026-09-18 15:52   ` Harry Yoo
2026-09-21  4:36     ` Hao Li
2026-09-21  4:21   ` Hao Li [this message]

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=arCeRr44nni8hykr@fedora \
    --to=hao.li@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=harry@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.