Re: [PATCH 1/2] alloc_tag: avoid current->alloc_tag manipulations when profiling is disabled

[Suren Baghdasaryan <surenb@google.com>]

On Thu, Dec 26, 2024 at 5:09 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Thu, Dec 26, 2024 at 5:07 PM Suren Baghdasaryan <surenb@google.com> wrote:
> >
> > On Thu, Dec 26, 2024 at 4:43 PM Kent Overstreet
> > <kent.overstreet@linux.dev> wrote:
> > >
> > > On Thu, Dec 26, 2024 at 01:16:38PM -0800, Suren Baghdasaryan wrote:
> > > > When memory allocation profiling is disabled there is no need to update
> > > > current->alloc_tag and these manipulations add unnecessary overhead. Fix
> > > > the overhead by skipping these extra updates.
> > >
> > > I did it the other way because I was concerned about the overhead of
> > > adding a huge number of static keys. But on further thought a static key
> > > probably isn't any bigger than an alloc tag, no?
> >
> > Hmm but a call
>
> Sorry, I pressed enter before finishing typing.
>
> Is a call to static_branch_maybe() generate a new static key? I
> thought mem_alloc_profiling_key would be used everywhere but maybe I'm
> missing something?

Or maybe you meant the NoOp/Jump generated for a static_branch ?

>
> >
> > >
> > > >
> > > > Fixes: b951aaff5035 ("mm: enable page allocation tagging")
> > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > > Cc: stable@vger.kernel.org
> > > > ---
> > > >  include/linux/alloc_tag.h | 11 ++++++++---
> > > >  lib/alloc_tag.c           |  2 ++
> > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> > > > index 0bbbe537c5f9..a946e0203e6d 100644
> > > > --- a/include/linux/alloc_tag.h
> > > > +++ b/include/linux/alloc_tag.h
> > > > @@ -224,9 +224,14 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes) {}
> > > >
> > > >  #define alloc_hooks_tag(_tag, _do_alloc)                             \
> > > >  ({                                                                   \
> > > > -     struct alloc_tag * __maybe_unused _old = alloc_tag_save(_tag);  \
> > > > -     typeof(_do_alloc) _res = _do_alloc;                             \
> > > > -     alloc_tag_restore(_tag, _old);                                  \
> > > > +     typeof(_do_alloc) _res;                                         \
> > > > +     if (mem_alloc_profiling_enabled()) {                            \
> > > > +             struct alloc_tag * __maybe_unused _old;                 \
> > > > +             _old = alloc_tag_save(_tag);                            \
> > > > +             _res = _do_alloc;                                       \
> > > > +             alloc_tag_restore(_tag, _old);                          \
> > > > +     } else                                                          \
> > > > +             _res = _do_alloc;                                       \
> > > >       _res;                                                           \
> > > >  })
> > > >
> > > > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > > > index 7dcebf118a3e..4c373f444eb1 100644
> > > > --- a/lib/alloc_tag.c
> > > > +++ b/lib/alloc_tag.c
> > > > @@ -29,6 +29,8 @@ EXPORT_SYMBOL(_shared_alloc_tag);
> > > >
> > > >  DEFINE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
> > > >                       mem_alloc_profiling_key);
> > > > +EXPORT_SYMBOL(mem_alloc_profiling_key);
> > > > +
> > > >  DEFINE_STATIC_KEY_FALSE(mem_profiling_compressed);
> > > >
> > > >  struct alloc_tag_kernel_section kernel_tags = { NULL, 0 };
> > > >
> > > > base-commit: 431614f1580a03c1a653340c55ea76bd12a9403f
> > > > --
> > > > 2.47.1.613.gc27f4b7a9f-goog
> > > >