| Message ID | 20241226211639.1357704-1-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] alloc_tag: avoid current->alloc_tag manipulations when profiling is disabled | expand |
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? > > 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 >
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 > > > > > 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 > >
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? > > > > > > > > > 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 > > >
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 > > > >
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 };
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. 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(-) base-commit: 431614f1580a03c1a653340c55ea76bd12a9403f