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

[Vlastimil Babka <vbabka@suse.cz>]

On 1/13/26 2:32 PM, Harry Yoo wrote:
> On Tue, Jan 13, 2026 at 10:01:16PM +0900, Harry Yoo wrote:
>> On Tue, Jan 13, 2026 at 01:50:31PM +0100, Vlastimil Babka wrote:
>>> On 1/13/26 7:18 AM, Harry Yoo wrote:
>>>
>>> Does this look OK to you or was there a reason you didn't do it? :)
>>>
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index ba15df4ca417..deb69bd9646a 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -981,8 +981,7 @@ static inline bool obj_exts_in_slab(struct kmem_cache *s, struct slab *slab)
>>>  #if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
>>>  static bool obj_exts_in_object(struct kmem_cache *s, struct slab *slab)
>>>  {
>>> -       return obj_exts_in_slab(s, slab) &&
>>> -              (slab_get_stride(slab) == s->size);
>>> +       return obj_exts_in_slab(s, slab) && (s->flags & SLAB_OBJ_EXT_IN_OBJ);
>>
>> There was a reason why I didn't do it :)
>>
>> In alloc_slab_obj_exts_early(), when both
>> obj_exts_fit_within_slab_leftover() and (s->flags & SLAB_OBJ_EXT_IN_OBJ)
>> returns true, it allocates the metadata from the slab's leftover space.
>>
>> I noticed it as I saw a slab error in slab_pad_check() complaining that
>> the padding area was overwritten, but turned out the problem was
>> because obj_exts_in_object() returning true when it shouldn't.
> 
> Perhaps a comment like this?
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index ba15df4ca417..c40c3559039e 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -981,6 +981,15 @@ static inline bool obj_exts_in_slab(struct kmem_cache *s, struct slab *slab)
>  #if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
>  static bool obj_exts_in_object(struct kmem_cache *s, struct slab *slab)
>  {
> +	/*
> +	 * When SLAB_OBJ_EXT_IN_OBJ is set, slabobj_ext metadata can be stored
> +	 * in one of two ways:
> +	 * 1. As an array in the slab's leftover space (after the last object)
> +	 * 2. Inline with each object (within s->size)
> +	 *
> +	 * The actual placement is determined by the stride size rather than
> +	 * the SLAB_OBJ_EXT_IN_OBJ flag itself.
> +	 */
>  	return obj_exts_in_slab(s, slab) &&
>  	       (slab_get_stride(slab) == s->size);
>  }

I meanwhile wrote this one. I think the part about depending on slab's size
is important so one doesn't wonder why we don't simply clear SLAB_OBJ_EXT_IN_OBJ
if it fits within_slab_leftover. As discussed off-list, will use it. Thanks!

--- a/mm/slub.c
+++ b/mm/slub.c
@@ -981,6 +981,12 @@ static inline bool obj_exts_in_slab(struct kmem_cache *s, struct slab *slab)
 #if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
 static bool obj_exts_in_object(struct kmem_cache *s, struct slab *slab)
 {
+       /*
+        * Note we cannot rely on the SLAB_OBJ_EXT_IN_OBJ flag here and need to
+        * check the stride. A cache can have SLAB_OBJ_EXT_IN_OBJ set, but
+        * allocations within_slab_leftover are preferred. And those may be
+        * possible or not depending on the particular slab's size.
+        */
        return obj_exts_in_slab(s, slab) &&
               (slab_get_stride(slab) == s->size);
 }