linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Suren Baghdasaryan <surenb@google.com>
To: Usama Arif <usamaarif642@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, hannes@cmpxchg.org,  shakeel.butt@linux.dev,
	linux-kernel@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH] alloc_tag: introduce Kconfig option for default compressed profiling
Date: Wed, 16 Apr 2025 14:08:31 -0700	[thread overview]
Message-ID: <CAJuCfpEKrX+1_SJ5fOyT6JLDSNcDxjcfBMj9_siVZt-rX5WQ=w@mail.gmail.com> (raw)
In-Reply-To: <20250416180653.3438158-1-usamaarif642@gmail.com>

On Wed, Apr 16, 2025 at 11:06 AM Usama Arif <usamaarif642@gmail.com> wrote:
>
> With this Kconfig option enabled, the kernel stores allocation tag references
> in the page flags by default.
>
> There are 2 reasons to introduce this:
> - As mentioned in [1], compressed tags dont have system memory overhead
> and much lower performance overhead. It would be preferrable to have this as
> the default option, and to be able to switch it at compile time. Another
> option is to just declare the static key as true by default?
> - As compressed option is the best one, it doesn't make sense to have to
> change both defconfig and command line options to enable memory
> allocation profiling. Changing commandline across a large number of services
> can result in signifcant work, which shouldn't be needed if the kernel
> defconfig needs to be changed anyways.

The reason tag compression is not the default option is because it
works only if there are enough free bits in the page flags to store a
tag index. If you configure it to use page flags and your build does
not have enough free bits, the profiling will be disabled (see
alloc_tag_sec_init()). IOW there is no graceful fallback to use page
extensions. Therefore, the current default option is not the most
performant but the one which works on all builds. Instead of this just
set sysctl.vm.mem_profiling boot parameter in your config file.

Your change effectively changes the default value of
mem_profiling_compressed and I don't see why you need to introduce a
new config option for that. But that really does not matter because
changing default to compressed tags is not the right choice IMO.

>
> [1] https://lore.kernel.org/all/20241023170759.999909-7-surenb@google.com/T/#m0da08879435f7673eaa10871a6e9d1be4f605ac8
>
> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
> ---
>  include/linux/pgalloc_tag.h | 4 ++++
>  lib/Kconfig.debug           | 5 +++++
>  lib/alloc_tag.c             | 4 ++++
>  3 files changed, 13 insertions(+)
>
> diff --git a/include/linux/pgalloc_tag.h b/include/linux/pgalloc_tag.h
> index c74077977830..0226059bcf00 100644
> --- a/include/linux/pgalloc_tag.h
> +++ b/include/linux/pgalloc_tag.h
> @@ -16,7 +16,11 @@ extern unsigned long alloc_tag_ref_mask;
>  extern int alloc_tag_ref_offs;
>  extern struct alloc_tag_kernel_section kernel_tags;
>
> +#ifdef CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT
> +DECLARE_STATIC_KEY_TRUE(mem_profiling_compressed);
> +#else
>  DECLARE_STATIC_KEY_FALSE(mem_profiling_compressed);
> +#endif
>
>  typedef u16    pgalloc_tag_idx;
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9fe4d8dfe578..66d8995f3514 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1028,6 +1028,11 @@ config MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT
>         default y
>         depends on MEM_ALLOC_PROFILING
>
> +config MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT
> +       bool "store page allocation tag references in the page flags by default"
> +       default y
> +       depends on MEM_ALLOC_PROFILING
> +
>  config MEM_ALLOC_PROFILING_DEBUG
>         bool "Memory allocation profiler debugging"
>         default n
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 25ecc1334b67..30adad5630dd 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -31,7 +31,11 @@ DEFINE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
>                         mem_alloc_profiling_key);
>  EXPORT_SYMBOL(mem_alloc_profiling_key);
>
> +#ifdef CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT
> +DEFINE_STATIC_KEY_TRUE(mem_profiling_compressed);
> +#else
>  DEFINE_STATIC_KEY_FALSE(mem_profiling_compressed);
> +#endif
>
>  struct alloc_tag_kernel_section kernel_tags = { NULL, 0 };
>  unsigned long alloc_tag_ref_mask;
> --
> 2.47.1
>


  reply	other threads:[~2025-04-16 21:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-16 18:06 Usama Arif
2025-04-16 21:08 ` Suren Baghdasaryan [this message]
2025-04-16 21:41   ` Shakeel Butt
2025-04-17  0:11     ` Suren Baghdasaryan
2025-04-17 15:47       ` Shakeel Butt
2025-04-17 16:00         ` Suren Baghdasaryan
2025-04-17 17:50           ` Usama Arif
2025-04-17 18:35             ` Usama Arif
2025-04-17 18:38               ` Suren Baghdasaryan
2025-04-16 21:52   ` Usama Arif
2025-04-17  0:15     ` Suren Baghdasaryan
2025-04-17 14:33       ` Usama Arif

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJuCfpEKrX+1_SJ5fOyT6JLDSNcDxjcfBMj9_siVZt-rX5WQ=w@mail.gmail.com' \
    --to=surenb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shakeel.butt@linux.dev \
    --cc=usamaarif642@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox