| Message ID | 20240221194052.927623-16-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Memory allocation profiling | expand |
On 2/21/24 20:40, Suren Baghdasaryan wrote: > Introduce helper functions to easily instrument page allocators by > storing a pointer to the allocation tag associated with the code that > allocated the page in a page_ext field. > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > Co-developed-by: Kent Overstreet <kent.overstreet@linux.dev> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> The static key usage seems fine now. Even if the page_ext overhead is still always paid when compiled in, you mention in the cover letter there's a plan for boot-time toggle later, so Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
On Mon, Feb 26, 2024 at 9:07 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 2/21/24 20:40, Suren Baghdasaryan wrote: > > Introduce helper functions to easily instrument page allocators by > > storing a pointer to the allocation tag associated with the code that > > allocated the page in a page_ext field. > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > Co-developed-by: Kent Overstreet <kent.overstreet@linux.dev> > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > The static key usage seems fine now. Even if the page_ext overhead is still > always paid when compiled in, you mention in the cover letter there's a plan > for boot-time toggle later, so Yes, I already have a simple patch for that to be included in the next revision: https://github.com/torvalds/linux/commit/7ca367e80232345f471b77b3ea71cf82faf50954 > > Reviewed-by: Vlastimil Babka <vbabka@suse.cz> Thanks! > >
On 2/26/24 18:11, Suren Baghdasaryan wrote: > On Mon, Feb 26, 2024 at 9:07 AM Vlastimil Babka <vbabka@suse.cz> wrote: >> >> On 2/21/24 20:40, Suren Baghdasaryan wrote: >>> Introduce helper functions to easily instrument page allocators by >>> storing a pointer to the allocation tag associated with the code that >>> allocated the page in a page_ext field. >>> >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> >>> Co-developed-by: Kent Overstreet <kent.overstreet@linux.dev> >>> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> >> >> The static key usage seems fine now. Even if the page_ext overhead is still >> always paid when compiled in, you mention in the cover letter there's a plan >> for boot-time toggle later, so > > Yes, I already have a simple patch for that to be included in the next > revision: https://github.com/torvalds/linux/commit/7ca367e80232345f471b77b3ea71cf82faf50954 This opt-out logic would require a distro kernel with allocation profiling compiled-in to ship together with something that modifies kernel command line to disable it by default, so it's not very practical. Could the CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT be turned into having 3 possible choices, where one of them would initialize mem_profiling_enabled to false? Or, taking a step back, is it going to be a common usecase to pay the memory overhead unconditionally, but only enable the profiling later during runtime? Also what happens if someone would enable and disable it multiple times during one boot? Would the statistics get all skewed because some frees would be not accounted while it's disabled? >> >> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > > Thanks! > >> >>
On Tue, Feb 27, 2024 at 10:30:53AM +0100, Vlastimil Babka wrote: > > > On 2/26/24 18:11, Suren Baghdasaryan wrote: > > On Mon, Feb 26, 2024 at 9:07 AM Vlastimil Babka <vbabka@suse.cz> wrote: > >> > >> On 2/21/24 20:40, Suren Baghdasaryan wrote: > >>> Introduce helper functions to easily instrument page allocators by > >>> storing a pointer to the allocation tag associated with the code that > >>> allocated the page in a page_ext field. > >>> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> > >>> Co-developed-by: Kent Overstreet <kent.overstreet@linux.dev> > >>> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > >> > >> The static key usage seems fine now. Even if the page_ext overhead is still > >> always paid when compiled in, you mention in the cover letter there's a plan > >> for boot-time toggle later, so > > > > Yes, I already have a simple patch for that to be included in the next > > revision: https://github.com/torvalds/linux/commit/7ca367e80232345f471b77b3ea71cf82faf50954 > > This opt-out logic would require a distro kernel with allocation > profiling compiled-in to ship together with something that modifies > kernel command line to disable it by default, so it's not very > practical. Could the CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT be > turned into having 3 possible choices, where one of them would > initialize mem_profiling_enabled to false? > > Or, taking a step back, is it going to be a common usecase to pay the > memory overhead unconditionally, but only enable the profiling later > during runtime? Also what happens if someone would enable and disable it > multiple times during one boot? Would the statistics get all skewed > because some frees would be not accounted while it's disabled? I already wrote the code for fast lookup from codetag index -> codetag - i.e. pointer compression - so this is all going away shortly. It just won't be in the initial pull request because of other dependencies (it requires my eytzinger code, which I was already lifting from fs/bcachefs/ for 6.9), but it can still probably make 6.9 in a second smaller pull.
On Tue, Feb 27, 2024 at 1:30 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > On 2/26/24 18:11, Suren Baghdasaryan wrote: > > On Mon, Feb 26, 2024 at 9:07 AM Vlastimil Babka <vbabka@suse.cz> wrote: > >> > >> On 2/21/24 20:40, Suren Baghdasaryan wrote: > >>> Introduce helper functions to easily instrument page allocators by > >>> storing a pointer to the allocation tag associated with the code that > >>> allocated the page in a page_ext field. > >>> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> > >>> Co-developed-by: Kent Overstreet <kent.overstreet@linux.dev> > >>> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > >> > >> The static key usage seems fine now. Even if the page_ext overhead is still > >> always paid when compiled in, you mention in the cover letter there's a plan > >> for boot-time toggle later, so > > > > Yes, I already have a simple patch for that to be included in the next > > revision: https://github.com/torvalds/linux/commit/7ca367e80232345f471b77b3ea71cf82faf50954 > > This opt-out logic would require a distro kernel with allocation > profiling compiled-in to ship together with something that modifies > kernel command line to disable it by default, so it's not very > practical. Could the CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT be > turned into having 3 possible choices, where one of them would > initialize mem_profiling_enabled to false? I was thinking about a similar approach of having the early boot parameter to be a tri-state with "0 | 1 | Never". The default option would be "Never" if CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT=n and "1" if CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT=y. Would that solve the problem for distributions? > > Or, taking a step back, is it going to be a common usecase to pay the > memory overhead unconditionally, but only enable the profiling later > during runtime? I think that would be the option one would use in the early deployments, to be able to enable the feature on specific devices without a reboot. Pasha brought up also an option when we disable the feature initially (via early boot option) but can enable it and reboot the system that will come up with enabled option. As Kent mentioned, he has been working on a pointer compression mechanism to cut the overhead of each codtag reference from one pointer (8 bytes) to 2 bytes index. I'm yet to check the performance but if that works and we can fit this index into page flags, that would completely eliminate dependency on page_ext and this memory overhead will be gone. This mechanism is not mature enough and I don't want to include these optimizations into the initial patchset, that's why it's not included in this patchset. > Also what happens if someone would enable and disable it > multiple times during one boot? Would the statistics get all skewed > because some frees would be not accounted while it's disabled? Yes and this was discussed during last LSFMM when the runtime control was brought up for the first time. That loss of accounting while the feature is disabled seems to be expected and acceptable. One could snapshot the state before re-enabling the feature and then compare later results with the initial snapshot to figure out the allocation growth. > > >> > >> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > > > > Thanks! > > > >> > >> > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h index be98564191e6..07e0656898f9 100644 --- a/include/linux/page_ext.h +++ b/include/linux/page_ext.h @@ -4,7 +4,6 @@ #include <linux/types.h> #include <linux/stacktrace.h> -#include <linux/stackdepot.h> struct pglist_data; diff --git a/include/linux/pgalloc_tag.h b/include/linux/pgalloc_tag.h new file mode 100644 index 000000000000..b49ab955300f --- /dev/null +++ b/include/linux/pgalloc_tag.h @@ -0,0 +1,78 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * page allocation tagging + */ +#ifndef _LINUX_PGALLOC_TAG_H +#define _LINUX_PGALLOC_TAG_H + +#include <linux/alloc_tag.h> + +#ifdef CONFIG_MEM_ALLOC_PROFILING + +#include <linux/page_ext.h> + +extern struct page_ext_operations page_alloc_tagging_ops; +extern struct page_ext *page_ext_get(struct page *page); +extern void page_ext_put(struct page_ext *page_ext); + +static inline union codetag_ref *codetag_ref_from_page_ext(struct page_ext *page_ext) +{ + return (void *)page_ext + page_alloc_tagging_ops.offset; +} + +static inline struct page_ext *page_ext_from_codetag_ref(union codetag_ref *ref) +{ + return (void *)ref - page_alloc_tagging_ops.offset; +} + +/* Should be called only if mem_alloc_profiling_enabled() */ +static inline union codetag_ref *get_page_tag_ref(struct page *page) +{ + if (page) { + struct page_ext *page_ext = page_ext_get(page); + + if (page_ext) + return codetag_ref_from_page_ext(page_ext); + } + return NULL; +} + +static inline void put_page_tag_ref(union codetag_ref *ref) +{ + page_ext_put(page_ext_from_codetag_ref(ref)); +} + +static inline void pgalloc_tag_add(struct page *page, struct task_struct *task, + unsigned int order) +{ + if (mem_alloc_profiling_enabled()) { + union codetag_ref *ref = get_page_tag_ref(page); + + if (ref) { + alloc_tag_add(ref, task->alloc_tag, PAGE_SIZE << order); + put_page_tag_ref(ref); + } + } +} + +static inline void pgalloc_tag_sub(struct page *page, unsigned int order) +{ + if (mem_alloc_profiling_enabled()) { + union codetag_ref *ref = get_page_tag_ref(page); + + if (ref) { + alloc_tag_sub(ref, PAGE_SIZE << order); + put_page_tag_ref(ref); + } + } +} + +#else /* CONFIG_MEM_ALLOC_PROFILING */ + +static inline void pgalloc_tag_add(struct page *page, struct task_struct *task, + unsigned int order) {} +static inline void pgalloc_tag_sub(struct page *page, unsigned int order) {} + +#endif /* CONFIG_MEM_ALLOC_PROFILING */ + +#endif /* _LINUX_PGALLOC_TAG_H */ diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 78d258ca508f..7bbdb0ddb011 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -978,6 +978,7 @@ config MEM_ALLOC_PROFILING depends on PROC_FS depends on !DEBUG_FORCE_WEAK_PER_CPU select CODE_TAGGING + select PAGE_EXTENSION help Track allocation source code and record total allocation size initiated at that code location. The mechanism can be used to track diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c index f09c8a422bc2..cb5adec4b2e2 100644 --- a/lib/alloc_tag.c +++ b/lib/alloc_tag.c @@ -3,6 +3,7 @@ #include <linux/fs.h> #include <linux/gfp.h> #include <linux/module.h> +#include <linux/page_ext.h> #include <linux/proc_fs.h> #include <linux/seq_buf.h> #include <linux/seq_file.h> @@ -115,6 +116,22 @@ static bool alloc_tag_module_unload(struct codetag_type *cttype, return module_unused; } +static __init bool need_page_alloc_tagging(void) +{ + return true; +} + +static __init void init_page_alloc_tagging(void) +{ +} + +struct page_ext_operations page_alloc_tagging_ops = { + .size = sizeof(union codetag_ref), + .need = need_page_alloc_tagging, + .init = init_page_alloc_tagging, +}; +EXPORT_SYMBOL(page_alloc_tagging_ops); + static struct ctl_table memory_allocation_profiling_sysctls[] = { { .procname = "mem_profiling", diff --git a/mm/mm_init.c b/mm/mm_init.c index 2c19f5515e36..e9ea2919d02d 100644 --- a/mm/mm_init.c +++ b/mm/mm_init.c @@ -24,6 +24,7 @@ #include <linux/page_ext.h> #include <linux/pti.h> #include <linux/pgtable.h> +#include <linux/stackdepot.h> #include <linux/swap.h> #include <linux/cma.h> #include <linux/crash_dump.h> diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 150d4f23b010..edb79a55a252 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -53,6 +53,7 @@ #include <linux/khugepaged.h> #include <linux/delayacct.h> #include <linux/cacheinfo.h> +#include <linux/pgalloc_tag.h> #include <asm/div64.h> #include "internal.h" #include "shuffle.h" @@ -1100,6 +1101,7 @@ static __always_inline bool free_pages_prepare(struct page *page, /* Do not let hwpoison pages hit pcplists/buddy */ reset_page_owner(page, order); page_table_check_free(page, order); + pgalloc_tag_sub(page, order); return false; } @@ -1139,6 +1141,7 @@ static __always_inline bool free_pages_prepare(struct page *page, page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP; reset_page_owner(page, order); page_table_check_free(page, order); + pgalloc_tag_sub(page, order); if (!PageHighMem(page)) { debug_check_no_locks_freed(page_address(page), @@ -1532,6 +1535,7 @@ inline void post_alloc_hook(struct page *page, unsigned int order, set_page_owner(page, order, gfp_flags); page_table_check_alloc(page, order); + pgalloc_tag_add(page, current, order); } static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags, diff --git a/mm/page_ext.c b/mm/page_ext.c index 4548fcc66d74..3c58fe8a24df 100644 --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -10,6 +10,7 @@ #include <linux/page_idle.h> #include <linux/page_table_check.h> #include <linux/rcupdate.h> +#include <linux/pgalloc_tag.h> /* * struct page extension @@ -82,6 +83,9 @@ static struct page_ext_operations *page_ext_ops[] __initdata = { #if defined(CONFIG_PAGE_IDLE_FLAG) && !defined(CONFIG_64BIT) &page_idle_ops, #endif +#ifdef CONFIG_MEM_ALLOC_PROFILING + &page_alloc_tagging_ops, +#endif #ifdef CONFIG_PAGE_TABLE_CHECK &page_table_check_ops, #endif