All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Ge <hao.ge@linux.dev>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Abhishek Bapat <abhishekbapat@google.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
Date: Wed, 5 Aug 2026 10:51:39 +0800	[thread overview]
Message-ID: <b2684958-5c39-4a9f-ab83-2e665406c0c0@linux.dev> (raw)
In-Reply-To: <bd95f8c6-33e2-4fda-bd4b-ebc37ee7c0fb@linux.dev>


On 2026/8/5 10:26, Hao Ge wrote:
> Hi Suren
>
>
> Thanks a lot for your review.
>
>
> On 2026/8/5 04:08, Suren Baghdasaryan wrote:
>> On Tue, Aug 4, 2026 at 5:21 AM Hao Ge <hao.ge@linux.dev> wrote:
>>> In reserve_module_tags(), the tag overflow check is gated on
>>> mem_alloc_profiling_enabled():
>>>
>>>      if (mem_alloc_profiling_enabled() && !tags_addressable())
>>>
>>> If profiling is toggled off at runtime and a module is loaded whose
>>> tags exceed the compressed-mode limit, shutdown_mem_profiling() is
>>> skipped. vm_module_tags_populate() still maps memory for the tags and
>>> the module loads successfully, but the total tag count now exceeds what
>>> NR_UNUSED_PAGEFLAG_BITS can address.
>>>
>>> Once profiling is re-enabled, ref_to_idx() computes each tag's index
>>> as its position in the alloc_tag array. update_page_tag_ref() masks
>>> it to alloc_tag_ref_mask before storing in page->flags. Indices
>>> beyond the mask are truncated and idx_to_ref() resolves them to wrong
>>> tags.
>>>
>>> mem_alloc_profiling_enabled() and mem_profiling_compressed are
>>> independent. Once compressed mode is established at boot, it stays
>>> active regardless of runtime toggles of mem_profiling.
>>>
>>> Remove the mem_alloc_profiling_enabled() guard. Also return an error
>>> after shutdown_mem_profiling() to skip vm_module_tags_populate(), as
>>> the mapped pages would never be reused - shutdown_mem_profiling() sets
>>> mem_profiling_support to false, so no future module load enters the
>>> codetag path.
>>>
>>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag 
>>> compression")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> Thanks for the fix, Hao!
>>
>>> ---
>>> Changes in v2:
>>> - Return error after shutdown_mem_profiling() to skip unnecessary
>>>    vm_module_tags_populate()
>>>    v1 link: 
>>> https://lore.kernel.org/all/20260804064408.105033-1-hao.ge@linux.dev/
>>> ---
>>>   mm/alloc_tag.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
>>> index 52aece27b00e..d8c36430f1c0 100644
>>> --- a/mm/alloc_tag.c
>>> +++ b/mm/alloc_tag.c
>>> @@ -904,10 +904,11 @@ 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()) {
>> Makes sense but how about replacing mem_alloc_profiling_enabled() with
>> mem_profiling_support? Otherwise this warning will be issued multiple
>> times if we are loading multiple modules.
>
>
> That's a really good point, this duplicate warning issue will indeed 
> happen.
>
> When I thought over your point, I realized using mem_profiling_support 
> creates a small race window.
>
>
> Thread A(insmod A)                      Thread B (insmod B)
>
> --------- ---------
>
> needs_section_mem() -> true
>
>     needs_section_mem() -> true
>
>     (profiling still supported)
>
> reserve_module_tags()
>
>     overflow -> shutdown
>
>     mem_profiling_support=false
>
>     return -ENOSPC
>
>     reserve_module_tags()
>
>         mem_profiling_support==false
>
>         -> overflow check skipped
>
>         -> vm_module_tags_populate()
>
>             maps unused tag pages


Sorry, my mail client messed up the race diagram, resend it.

Thread A (insmod A)                   Thread B (insmod B)
---------------------                 ---------------------
needs_section_mem() -> true
                                       needs_section_mem() -> true
                                       (profiling still supported)
reserve_module_tags()
   overflow -> shutdown_mem_profiling()
   mem_profiling_support = false
   return -ENOMEM
                                       reserve_module_tags()
                                         mem_profiling_support == false
                                         -> overflow check skipped
                                         -> vm_module_tags_populate()
                                            maps unused tag pages

>
> So I'd rather go with pr_warn_once here.
>
>
>>> +               if (!tags_addressable()) {
>>>                          shutdown_mem_profiling(true);
>>>                          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);
>>> +                       return ERR_PTR(-ENOSPC);
>> This ENOSPC error will be propagated all the way up to the init_module
>> syscall and it's not among the error codes currently expected (see:
>> https://man7.org/linux/man-pages/man2/init_module.2.html). I suggest
>> returning ENOMEM instead.
>
>
> Agree, will change
>
>
> Thanks
>
> Best Regards
>
> Hao
>
>>>                  }
>>>
>>>                  grow_res = vm_module_tags_populate();
>>> -- 
>>> 2.25.1
>>>


  reply	other threads:[~2026-08-05  2:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 12:20 [PATCH v2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-04 19:43 ` Andrew Morton
2026-08-05  8:35   ` Hao Ge
2026-08-04 20:08 ` Suren Baghdasaryan
2026-08-05  2:26   ` Hao Ge
2026-08-05  2:51     ` Hao Ge [this message]
2026-08-05 14:46     ` Suren Baghdasaryan

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=b2684958-5c39-4a9f-ab83-2e665406c0c0@linux.dev \
    --to=hao.ge@linux.dev \
    --cc=abhishekbapat@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.