Re: [RFC PATCH] mm/slab: save memory by allocating slabobj_ext array from leftover

[Harry Yoo <harry.yoo@oracle.com>]

On Thu, Jun 19, 2025 at 09:56:59AM +0200, Vlastimil Babka wrote:
> On 6/13/25 19:47, Harry Yoo wrote:
> > On Fri, Jun 13, 2025 at 09:04:34AM -0700, Christoph Lameter (Ampere) wrote:
> >> On Fri, 13 Jun 2025, Harry Yoo wrote:
> >> 
> >> > Allocate slabobj_exts array from this unused space instead of using
> >> > kcalloc(), when it is large enough.
> >> 
> >> How does slab debug work in this case? The object layout gets a bit
> >> complicated with other metadata there as well.
> > 
> > Oh, the 'leftover' space I mentioned the cover letter refers to the
> > wasted space after the last object in a slab, not unused bytes within
> > objects.
> > 
> > There is no per-object metadata stored there and SLUB simply poisons the area.
> > I taught slab_pad_check() to skip checking the slabobj_exts array.

Apologies for the late reply. I was sidetracked with multiple things :(

This is definitely worth optimizing, so let me make some progress
even if it's a bit slow.

> I can imagine going further with this in case where leftover space in slab
> isn't enough.

Right.

> - indeed use per-object padding to store only single object's slabobj_ext,

I think the most conservative approach is to not increase object_size
but use wasted area when ALIGN(size, align) is bigger than object_size.

A good candidate for that is xfs inode cache.

> if it doesn't lead to memory waste

You mean increasing object size but without decreasing the number of
objects per slab?

...or (maybe) reducing the number of objects but without increasing
the size of the remainder (same as calculated in calc_slab_order())?

> - if not possible, but object size is small enough so there are many per
> slab, maybe have one less object per slab to store the array?

If object size is small the array likely does not fit in one object...

> - once we have struct slab decoupled from struct page, it could be part of
> struct slab directly (but it would mean struct slab isn't fixed size)

It can be tried, but variable struct slab size may or may not work.
That'll depend on the implementation details of how we allocate struct slab
in the future.

> Of course having multiple variants would risk slower code, so fast paths
> should not be affected 

I agree that affecting fastpath is not great.

> we could have pointer to the 0th slabobj_ext (we
> already have) and now also stride (to support the "per-object padding case"

> - there's still space in struct slab right?)

which space are you referring to, maybe lower 16 bits of page_type?

> and then the object alloc/free
> case could be oblivious to the storage method, with just a bit more
> arithmetic (stride). Slab folio alloc/free would be more complicated but are
> not fath path.

So it would be something like (please correct if I misunderstood):

index = obj_to_index(s, slab, object)
(the struct slabobj_ext pointer for the object at given index)
  == slab->obj_exts + stride * index

slab->obj_exts, stride are determined depending on the case:

- In the normal case (the array is allocated from kmalloc caches),
  stride = sizeof(struct slabobj_ext)
  slab->obj_exts = (the address of the buffer allocated from kmalloc)

- In "the obj_exts array is stored in the leftover space" case,
  stride = sizeof(struct slabobj_ext)
  slab->obj_exts = (the start address of the leftover space)

- In "per-object padding" case,
  stride = s->size
  slab->obj_exts = slab_address(slab) + s->red_left_pad +
                       (offset of slabobj_ext);

Ok, I think it will work. Great idea!

> Also some variants would be wasteful if they need to be decided upfront (the
> 2nd and 3rd above) and then the array is unused

Right.

> so would be only applicable
> with SLAB_ACCOUNT caches (if kmemcg is active) or when memalloc profiling is
> active.

Right.

> Shouldn't be a big issue as ad-hoc __GFP_ACCOUNT is handled by
> different cache selection for kmalloc() and I don't know if anyone is
> actually doing ad-hoc __GFP_ACCOUNT on named caches.

A while ago I was thinking of getting rid of ad-hoc __GFP_ACCOUNT usage
for slab allocations, but at least xarray cache appears to use it in
an ad-hoc manner. (See xas_nomem()).

In that case, not all allocations to the same cache has
__GFP_ACCOUNT flag set.

-- 
Cheers,
Harry / Hyeonggon