Re: [PATCH V4 8/8] mm/slab: place slabobj_ext metadata in unused space within s->size

[Hao Li <hao.li@linux.dev>]

On Tue, Dec 30, 2025 at 01:59:57PM +0900, Harry Yoo wrote:
> On Wed, Dec 24, 2025 at 08:43:17PM +0800, Hao Li wrote:
> > On Wed, Dec 24, 2025 at 03:38:57PM +0900, Harry Yoo wrote:
> > > On Wed, Dec 24, 2025 at 01:33:59PM +0800, Hao Li wrote:
> > > > One more thought: in calculate_sizes() we add some extra padding when
> > > > SLAB_RED_ZONE is enabled:
> > > > 
> > > > if (flags & SLAB_RED_ZONE) {
> > > > 	/*
> > > > 	 * Add some empty padding so that we can catch
> > > > 	 * overwrites from earlier objects rather than let
> > > > 	 * tracking information or the free pointer be
> > > > 	 * corrupted if a user writes before the start
> > > > 	 * of the object.
> > > > 	 */
> > > > 	size += sizeof(void *);
> > > > 	...
> > > > }
> > > > 
> > > > 
> > > > From what I understand, this additional padding ends up being placed
> > > > after the KASAN allocation metadata.
> > > 
> > > Right.
> > > 
> > > > Since it’s only "extra" padding (i.e., it doesn’t seem strictly required
> > > > for the layout), and your patch would reuse this area — together with
> > > > the final padding introduced by `size = ALIGN(size, s->align);`
> > > 
> > > Very good point!
> > > Nah, it wasn't intentional to reuse the extra padding.
> 
> Waaaait, now I'm looking into it again to write V5...
> 
> It may reduce (or remove) the space for the final padding but not the
> mandatory padding because the mandatory padding is already included
> in the size before `aligned_size = ALIGN(size, s->align)`

Ah, right - I double-checked as well. `aligned_size - size` is exactly the
space reserved for the final padding, so slabobj_ext won't eat into the
mandatory padding.

> 
> > > > for objext, it seems like this padding may no longer provide much benefit.
> > > > Do you think it would make sense to remove this extra padding
> > > > altogether?
> > > 
> > > I think when debugging flags are enabled it'd still be useful to have,
> > 
> > Absolutely — I’m with you on this.
> > 
> > After thinking about it again, I agree it’s better to keep it.
> > 
> > Without that mandatory extra word, we could end up with "no trailing
> > padding at all" in cases where ALIGN(size, s->align) doesn’t actually
> > add any bytes.
> > 
> > > I'll try to keep the padding area after obj_ext (so that overwrites from
> > > the previous object won't overwrite the metadata).
> > 
> > Agree — we should make sure there is at least sizeof(void *) of extra
> > space after obj_exts when SLAB_RED_ZONE is enabled, so POISON_INUSE has
> > somewhere to go.
> 
> I think V4 of the patchset is already doing that, no?
> 
> The mandatory padding exists after obj_ext if SLAB_RED_ZONE is enabled
> and the final padding may or may not exist. check_pad_bytes() already knows
> that the padding(s) exist after obj_ext.

Yes, you are right, V4 already does this — I just hadn't noticed it earlier...

> 
> By the way, thanks for fixing the comment once again,
> it's easier to think about the layout now.

Glad it helped. The object layout is really subtle — missing even a
small detail was enough to throw us off. Glad we finally got it all
straightened out.

-- 
Thanks,
Hao