Re: [PATCH v3 06/13] mm/slab: abstract slabobj_ext.ref access

Harry Yoo <[email protected]> Wed, 29 Jul 2026 06:18:44 +0000
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <ammZpX_77SMsxvPz@dev>
On Tue, Jul 28, 2026 at 07:41:15PM +0200, Vlastimil Babka (SUSE) wrote:
> On 7/28/26 15:41, Harry Yoo wrote:
> > On Mon, Jul 27, 2026 at 02:54:00PM +0200, Vlastimil Babka (SUSE) wrote:
> >> In preparation for changes to the structure, abstract access to the ref
> >> field with a slab_obj_ext_codetag_ref() function. Rename the field to
> >> _ctref to make an unexpected direct access a compile error.
> >> 
> >> No functional change intended.
> >> 
> >> Reviewed-by: Suren Baghdasaryan <[email protected]>
> >> Reviewed-by: Hao Li <[email protected]>
> >> Signed-off-by: Vlastimil Babka (SUSE) <[email protected]>
> >> ---
> > 
> > Looks good to me,
> > Reviewed-by: Harry Yoo (Oracle) <[email protected]>
> > 
> > With a suggestion: it would be nice to have debug warnings
> > that trigger when slab_obj_ext_codetag_ref() is compiled but
> > not supposed to be called.
> > 
> > e.g.) for kfence objects, or later in the series when we
> > reduce slabobj_ext memory (!slab_obj_ext_has_codetag()) 
> > 
> > Similarly, it would be nice (later in the series) to have a warning
> > in slab_obj_ext_objcg() when slab_needs_objcg() returns false.
> 
> OK good idea but we can't do it right now as the helpers are introduced
> later.
> 
> So for 9/13 mm/slab: introduce slab_obj_ext_has_codetag():
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index 8f352d9f4d91..fbad99da093c 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -726,6 +726,8 @@ static inline void slab_obj_ext_set_objcg(struct slabobj_ext *obj_ext,
>  static inline union codetag_ref *
>  slab_obj_ext_codetag_ref(struct slab *slab, struct slabobj_ext *obj_ext)
>  {
> +       VM_WARN_ON_ONCE(!slab_obj_ext_has_codetag());
> +
>         if (IS_ENABLED(CONFIG_MEMCG))
>                 obj_ext += 1;
>  
> ... and for objcg we would need to start passing slab pointer to
> slab_obj_ext_objcg() and slab_obj_ext_set_objcg(). Hmm...

But slab_obj_ext_codetag_ref() already takes the parameter to
calculate offset. I think it's worth as it'll be optimized away on
non-debug kerenels and there is no way to detect type confusion
due to subtle errors in the objext layout.