Cc Suren
Hi Christian
On 2026/11/5 03:06, Christian Brauner wrote:
> codetag_alloc_module_section() takes the codetag type's mod_lock for
> writing and calls reserve_module_tags() with it held. When that can't
> reserve the tags, because the section's alignment doesn't fit, because
> there are too many tags for the page flag bits or because the memory
> for them can't be allocated, it shuts memory allocation profiling down
> and removes /proc/allocinfo, still with mod_lock held.
>
> remove_proc_entry() waits until every reader of the entry has let go of
> it, and a reader of /proc/allocinfo takes mod_lock for reading in
> allocinfo_start() while it holds the entry. So a module load that hits
> one of those paths while /proc/allocinfo is being read waits for the
> reader and the reader waits for the module load, for good.
>
> Let a work item remove the entry in that case. The profiling is shut
> down right away like before, the file just goes a moment later.
>
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Cc: [email protected]
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>
Thanks.
is this a problem you have hit in practice?
Sashiko discovered this issue earlier and reported it as follows:
> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 2070e682fe106..112a014d4b890 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -950,10 +950,12 @@ static void *reserve_module_tags(struct module *mod,
> unsigned long size,
> int grow_res;
>
> module_tags.size = offset + size;
> - if (mem_alloc_profiling_enabled() && !tags_addressable()) {
> + if (!tags_addressable()) {
> shutdown_mem_profiling(true);
This is a pre-existing issue, but does this shutdown_mem_profiling(true) call
introduce an AB-BA deadlock between /proc/allocinfo file readers (pde->in_use)
and module loading (cttype->mod_lock)?
A user reading /proc/allocinfo acquires the procfs in_use reference in
use_pde(), then blocks on down_read(&alloc_tag_cttype->mod_lock) in
allocinfo_start().
Concurrently, if a module loads and tags_addressable() fails,
codetag_alloc_module_section() holds down_write(&cttype->mod_lock) and calls
shutdown_mem_profiling(true), which removes the proc entry. This removal waits
indefinitely for the in_use reference to drop to zero, hanging both procfs
reads and module loading.
> - pr_warn("With module %s there are too many tags to fit
> in %d page flag bits. Memory allocation profiling is disabled!\n",
> - mod->name, NR_UNUSED_PAGEFLAG_BITS);
> + pr_warn_once("With module %s there are too many tags to
> fit in %d page flag bits. Memory allocation profiling is disabled!\n",
> + mod->name, NR_UNUSED_PAGEFLAG_BITS);
> + release_module_tags(mod, false);
> + return ERR_PTR(-EAGAIN);
> }
>
> grow_res = vm_module_tags_populate();
I previously sent a patch with a comparable idea, though Sashiko remained
unconvinced.
[patch]: https://lore.kernel.org/all/[email protected]/
[Sashiko's objections]:
https://sashiko.dev/#/patchset/[email protected]
Could this patch perhaps wait for mine? No worries if this is inconvenient.
Best regards
Hao
> ---
> mm/alloc_tag.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 82e2c3448dcf..7d7ad6704953 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -15,6 +15,7 @@
> #include <linux/seq_file.h>
> #include <linux/string_choices.h>
> #include <linux/vmalloc.h>
> +#include <linux/workqueue.h>
> #include <linux/kmemleak.h>
> #include <uapi/linux/alloc_tag.h>
>
> @@ -604,6 +605,26 @@ static void shutdown_mem_profiling(bool remove_file)
> mem_profiling_support = false;
> }
>
> +static void remove_allocinfo_file(struct work_struct *work)
> +{
> + remove_proc_entry(ALLOCINFO_FILE_NAME, NULL);
> +}
> +static DECLARE_WORK(remove_allocinfo_work, remove_allocinfo_file);
> +
> +/*
> + * A module's tags are reserved with the codetag module lock held for
> + * writing, and a reader of /proc/allocinfo takes that lock with the entry
> + * pinned. Removing the entry there would wait for that reader for good, so
> + * let a work item do it.
> + */
> +static void shutdown_mem_profiling_async(void)
> +{
> + if (!mem_profiling_support)
> + return;
> + shutdown_mem_profiling(false);
> + schedule_work(&remove_allocinfo_work);
> +}
> +
> void __init alloc_tag_sec_init(void)
> {
> struct alloc_tag *last_codetag;
> @@ -855,7 +876,7 @@ static void *reserve_module_tags(struct module *mod,
> unsigned long size,
> align = 1;
>
> if (!ensure_alignment(align, &prepend)) {
> - shutdown_mem_profiling(true);
> + shutdown_mem_profiling_async();
> pr_err("%s: alignment %lu is incompatible with allocation tag
> indexing. Memory allocation profiling is disabled!\n",
> mod->name, align);
> return ERR_PTR(-EINVAL);
> @@ -905,14 +926,14 @@ unlock:
>
> module_tags.size = offset + size;
> if (mem_alloc_profiling_enabled() && !tags_addressable()) {
> - shutdown_mem_profiling(true);
> + shutdown_mem_profiling_async();
> pr_warn("With module %s there are too many tags to fit
> in %d page flag bits. Memory allocation profiling is disabled!\n",
> mod->name, NR_UNUSED_PAGEFLAG_BITS);
> }
>
> grow_res = vm_module_tags_populate();
> if (grow_res) {
> - shutdown_mem_profiling(true);
> + shutdown_mem_profiling_async();
> pr_err("Failed to allocate memory for allocation tags
> in the module %s. Memory allocation profiling is disabled!\n",
> mod->name);
> return ERR_PTR(grow_res);
>
