| Message ID | 20240614225951.3845577-1-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] mm/slab: fix 'variable obj_exts set but not used' warning | expand |
On 6/15/24 12:59 AM, Suren Baghdasaryan wrote: > slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain > slabobj_ext object. Currently the only user of slabobj_ext object in > this path is memory allocation profiling, therefore when it's not enabled > this object is not needed. This also generates a warning when compiling > with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration > to fix the warning. If more slabobj_ext users appear in the future, the > code will have to be changed back to call prepare_slab_obj_exts_hook(). > > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ > Signed-off-by: Suren Baghdasaryan <surenb@google.com> Acked-by: Vlastimil Babka <vbabka@suse.cz> But it seems to me we could remove the whole #ifdef if current->alloc_tag (which doesn't exist with !MEM_ALLOC_PROFILING) had an access helper, or there was a alloc_tag_add_current() variant? > --- > mm/slub.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 1373ac365a46..4927edec6a8c 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -3902,7 +3902,6 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > unsigned int orig_size) > { > unsigned int zero_size = s->object_size; > - struct slabobj_ext *obj_exts; > bool kasan_init = init; > size_t i; > gfp_t init_flags = flags & gfp_allowed_mask; > @@ -3945,9 +3944,11 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > kmemleak_alloc_recursive(p[i], s->object_size, 1, > s->flags, init_flags); > kmsan_slab_alloc(s, p[i], init_flags); > +#ifdef CONFIG_MEM_ALLOC_PROFILING > if (need_slab_obj_ext()) { > + struct slabobj_ext *obj_exts; > + > obj_exts = prepare_slab_obj_exts_hook(s, flags, p[i]); > -#ifdef CONFIG_MEM_ALLOC_PROFILING > /* > * Currently obj_exts is used only for allocation profiling. > * If other users appear then mem_alloc_profiling_enabled() > @@ -3955,8 +3956,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > */ > if (likely(obj_exts)) > alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size); > -#endif > } > +#endif > } > > return memcg_slab_post_alloc_hook(s, lru, flags, size, p); > > base-commit: c286c21ff94252f778515b21b6bebe749454a852
On Mon, Jun 17, 2024 at 3:04 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 6/15/24 12:59 AM, Suren Baghdasaryan wrote: > > slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain > > slabobj_ext object. Currently the only user of slabobj_ext object in > > this path is memory allocation profiling, therefore when it's not enabled > > this object is not needed. This also generates a warning when compiling > > with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration > > to fix the warning. If more slabobj_ext users appear in the future, the > > code will have to be changed back to call prepare_slab_obj_exts_hook(). > > > > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > > But it seems to me we could remove the whole #ifdef if current->alloc_tag > (which doesn't exist with !MEM_ALLOC_PROFILING) had an access helper, or > there was a alloc_tag_add_current() variant? Hmm. I'll check if current->alloc_tag is the only reason for this ifdef. If so then you are correct and we can simplify this code. > > > --- > > mm/slub.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/mm/slub.c b/mm/slub.c > > index 1373ac365a46..4927edec6a8c 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -3902,7 +3902,6 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > > unsigned int orig_size) > > { > > unsigned int zero_size = s->object_size; > > - struct slabobj_ext *obj_exts; > > bool kasan_init = init; > > size_t i; > > gfp_t init_flags = flags & gfp_allowed_mask; > > @@ -3945,9 +3944,11 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > > kmemleak_alloc_recursive(p[i], s->object_size, 1, > > s->flags, init_flags); > > kmsan_slab_alloc(s, p[i], init_flags); > > +#ifdef CONFIG_MEM_ALLOC_PROFILING > > if (need_slab_obj_ext()) { > > + struct slabobj_ext *obj_exts; > > + > > obj_exts = prepare_slab_obj_exts_hook(s, flags, p[i]); > > -#ifdef CONFIG_MEM_ALLOC_PROFILING > > /* > > * Currently obj_exts is used only for allocation profiling. > > * If other users appear then mem_alloc_profiling_enabled() > > @@ -3955,8 +3956,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, > > */ > > if (likely(obj_exts)) > > alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size); > > -#endif > > } > > +#endif > > } > > > > return memcg_slab_post_alloc_hook(s, lru, flags, size, p); > > > > base-commit: c286c21ff94252f778515b21b6bebe749454a852 >
On 6/30/24 9:20 PM, Suren Baghdasaryan wrote: > On Mon, Jun 17, 2024 at 3:04 AM Vlastimil Babka <vbabka@suse.cz> wrote: >> >> On 6/15/24 12:59 AM, Suren Baghdasaryan wrote: >> > slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain >> > slabobj_ext object. Currently the only user of slabobj_ext object in >> > this path is memory allocation profiling, therefore when it's not enabled >> > this object is not needed. This also generates a warning when compiling >> > with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration >> > to fix the warning. If more slabobj_ext users appear in the future, the >> > code will have to be changed back to call prepare_slab_obj_exts_hook(). >> > >> > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") >> > Reported-by: kernel test robot <lkp@intel.com> >> > Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ >> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> >> >> Acked-by: Vlastimil Babka <vbabka@suse.cz> >> >> But it seems to me we could remove the whole #ifdef if current->alloc_tag >> (which doesn't exist with !MEM_ALLOC_PROFILING) had an access helper, or >> there was a alloc_tag_add_current() variant? > > Hmm. I'll check if current->alloc_tag is the only reason for this > ifdef. If so then you are correct and we can simplify this code. The fix is now in mm-hotfixes-stable but we can cleanup for the future as a non-hotfix.
On Tue, Jul 2, 2024 at 9:31 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 6/30/24 9:20 PM, Suren Baghdasaryan wrote: > > On Mon, Jun 17, 2024 at 3:04 AM Vlastimil Babka <vbabka@suse.cz> wrote: > >> > >> On 6/15/24 12:59 AM, Suren Baghdasaryan wrote: > >> > slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain > >> > slabobj_ext object. Currently the only user of slabobj_ext object in > >> > this path is memory allocation profiling, therefore when it's not enabled > >> > this object is not needed. This also generates a warning when compiling > >> > with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration > >> > to fix the warning. If more slabobj_ext users appear in the future, the > >> > code will have to be changed back to call prepare_slab_obj_exts_hook(). > >> > > >> > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") > >> > Reported-by: kernel test robot <lkp@intel.com> > >> > Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ > >> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > >> > >> Acked-by: Vlastimil Babka <vbabka@suse.cz> > >> > >> But it seems to me we could remove the whole #ifdef if current->alloc_tag > >> (which doesn't exist with !MEM_ALLOC_PROFILING) had an access helper, or > >> there was a alloc_tag_add_current() variant? > > > > Hmm. I'll check if current->alloc_tag is the only reason for this > > ifdef. If so then you are correct and we can simplify this code. > > The fix is now in mm-hotfixes-stable but we can cleanup for the future as a > non-hotfix. Yes, it's on my TODO list now. Thanks! >
On Tue, Jul 2, 2024 at 8:16 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Tue, Jul 2, 2024 at 9:31 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > On 6/30/24 9:20 PM, Suren Baghdasaryan wrote: > > > On Mon, Jun 17, 2024 at 3:04 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > >> > > >> On 6/15/24 12:59 AM, Suren Baghdasaryan wrote: > > >> > slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain > > >> > slabobj_ext object. Currently the only user of slabobj_ext object in > > >> > this path is memory allocation profiling, therefore when it's not enabled > > >> > this object is not needed. This also generates a warning when compiling > > >> > with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration > > >> > to fix the warning. If more slabobj_ext users appear in the future, the > > >> > code will have to be changed back to call prepare_slab_obj_exts_hook(). > > >> > > > >> > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") > > >> > Reported-by: kernel test robot <lkp@intel.com> > > >> > Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ > > >> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > >> > > >> Acked-by: Vlastimil Babka <vbabka@suse.cz> > > >> > > >> But it seems to me we could remove the whole #ifdef if current->alloc_tag > > >> (which doesn't exist with !MEM_ALLOC_PROFILING) had an access helper, or > > >> there was a alloc_tag_add_current() variant? > > > > > > Hmm. I'll check if current->alloc_tag is the only reason for this > > > ifdef. If so then you are correct and we can simplify this code. > > > > The fix is now in mm-hotfixes-stable but we can cleanup for the future as a > > non-hotfix. > > Yes, it's on my TODO list now. Thanks! obj_exts->ref was also undefined when !MEM_ALLOC_PROFILING, so I moved that call into a separate hook. It's posted at https://lore.kernel.org/all/20240703015354.3370503-1-surenb@google.com/ Thanks, Suren. > > >
diff --git a/mm/slub.c b/mm/slub.c index 1373ac365a46..4927edec6a8c 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3902,7 +3902,6 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, unsigned int orig_size) { unsigned int zero_size = s->object_size; - struct slabobj_ext *obj_exts; bool kasan_init = init; size_t i; gfp_t init_flags = flags & gfp_allowed_mask; @@ -3945,9 +3944,11 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, kmemleak_alloc_recursive(p[i], s->object_size, 1, s->flags, init_flags); kmsan_slab_alloc(s, p[i], init_flags); +#ifdef CONFIG_MEM_ALLOC_PROFILING if (need_slab_obj_ext()) { + struct slabobj_ext *obj_exts; + obj_exts = prepare_slab_obj_exts_hook(s, flags, p[i]); -#ifdef CONFIG_MEM_ALLOC_PROFILING /* * Currently obj_exts is used only for allocation profiling. * If other users appear then mem_alloc_profiling_enabled() @@ -3955,8 +3956,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, */ if (likely(obj_exts)) alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size); -#endif } +#endif } return memcg_slab_post_alloc_hook(s, lru, flags, size, p);
slab_post_alloc_hook() uses prepare_slab_obj_exts_hook() to obtain slabobj_ext object. Currently the only user of slabobj_ext object in this path is memory allocation profiling, therefore when it's not enabled this object is not needed. This also generates a warning when compiling with CONFIG_MEM_ALLOC_PROFILING=n. Move the code under this configuration to fix the warning. If more slabobj_ext users appear in the future, the code will have to be changed back to call prepare_slab_obj_exts_hook(). Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202406150444.F6neSaiy-lkp@intel.com/ Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- mm/slub.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) base-commit: c286c21ff94252f778515b21b6bebe749454a852