Re: [PATCH] mm/slub: refill prefilled sheaves from the barn

[Harry Yoo <harry@kernel.org>]

On Fri, Sep 18, 2026 at 04:35:17PM +0100, Harry Yoo wrote:
> On Fri, Sep 18, 2026 at 07:41:56PM +0800, Hao Li wrote:
> > +/*
> > + * 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?
> Sounds like we're losing cache affinity unnecessarily.
> 
> I think we should try to refill from barn->sheaf_partial,
> or if that's not available, refill from a full sheaf, and then move
> the previously-full-sheaf to barn->sheaf_partial or barn->sheaf_empty.
> 
> Then we'll never replace the sheaf with a new one.
> 
> With that, the control flow could be simplified quite a bit.
> Something like this. (Warning: pseudocode, it won't compile)
> 
> // refill a sheaf from barn.
> // return true when the sheaf becomes full
> // return false when the sheaf is not full
> 
> bool refill_sheaf_from_barn(s, sheaf) {
> 	struct node_barn *barn = get_barn(s);
> 	struct slab_sheaf *partial;
> 	unsigned int to_move;
> 	unsigned long flags;
> 
> 	spin_lock_irqsave(&barn->lock, flags);
> 
> 	partial = barn->sheaf_partial;
> 	barn->sheaf_partial = NULL;
> 
> 	if (!partial && barn->nr_full) {
> 		// grab one from full list
> 		partial = [...];
> 	}
> 
> 	if (!partial)
> 		// cannot refill from the barn. the caller will try
> 		// refilling from n->partial list
> 		goto done;
> 
> 	to_move = min(s->sheaf_capacity - sheaf->size, partial->size);
> 	partial->size -= to_move;
> 	// copy `to_move` objects from `partial` to `sheaf`
> 	memcpy(...);
> 	sheaf->size += to_move;

Hmm, but if it's from barn->sheaf_partial, it might end up refilling
the sheaf from n->partial. Needs bit more thoughts. Perhaps retry if
it's still not full?

> 	if (!partial->size)
> 		// move it to empty list
> 	else
> 		barn->sheaf_partial = partial;
>
> done:
> 	spin_unlock_irqrestore(&barn->lock, flags);
> 	return (sheaf->size == s->sheaf_capacity);
> }
> 
> What do you think?
> 
> -- 
> Cheers,
> Harry / Hyeonggon

-- 
Cheers,
Harry / Hyeonggon