Re: [PATCH v3 15/16] mm: replace __GFP_NO_CODETAG with ALLOC_NO_CODETAG

[Hao Ge]

Hi Brendan


On 2026/6/30 18:10, Brendan Jackman wrote:
> On Tue Jun 30, 2026 at 1:55 AM UTC, Hao Ge wrote:
> > Hi Brendan
> >
> >
> > On 2026/6/29 21:12, Brendan Jackman wrote:
> > > Now that alloc_pages has an entrypoint that allows passing alloc_flags,
> > > we can take advantage of this to start removing GFP flags that are only
> > > used for mm-internal stuff.
> > >
> > > This requires also plumbing the alloc_flags into some more of the
> > > allocator code, in particular __alloc_pages[_noprof]() gets an
> > > alloc_flags arg to go along with its callees, and we now need to pass
> > > those flags deeper into the allocator so they can reach the alloc_tag
> > > code.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
> > > ---
> > >    mm/alloc_tag.c       | 22 ++++++----------------
> > >    mm/compaction.c      |  4 ++--
> > >    mm/internal.h        |  1 -
> > >    mm/page_alloc.c      | 42 ++++++++++++++++++++++++------------------
> > >    mm/page_alloc.h      | 17 +++++++++++++++--
> > >    mm/page_frag_cache.c |  4 ++--
> > >    6 files changed, 49 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> > > index d9be1cf5187d9..a32a94e759b94 100644
> > > --- a/mm/alloc_tag.c
> > > +++ b/mm/alloc_tag.c
> > > @@ -15,6 +15,8 @@
> > >    #include <linux/vmalloc.h>
> > >    #include <linux/kmemleak.h>
> > >    
> > > +#include "internal.h"
> >
> > Should we include page_alloc.h here, as we call __alloc_pages later in
> > this file?
> Yeah, there are a few build failures due to me not doing a broad enough
> build. From now on I will just wait for allmodconfig instead of trying
> to be clever with my build tests, sorry about this.


No worries at all. For the alloc_tag build error, it depends on whether

CONFIG_MEM_ALLOC_PROFILING_DEBUG is enabled.


> Also, this suggests that I have not actually re-tested the alloc_tag
> code since v3 so I must repeat the test described in my cover letter (I
> just manually enable the feature and check the kernel boots) for v4.

Thanks a lot for your work. I merged your patch locally via my 
automation bot,

and verified the basic functionality of alloc_tag.

The alloc_tag feature works well so far.


By the way, you may need to rebase your patchset on top of mm-new.

This patch has been merged into mm-new and will cause a minor conflict,

assuming the mm-new tree has not made further modifications to this patch:

https://lore.kernel.org/all/20260629-free-pfn-on-alloc-contig-range-error-path-v1-1-496ff9ca22db@xxxxxxxxxx/


Thanks

Best Regards

Hao


> > > +
> > >    #define ALLOCINFO_FILE_NAME		"allocinfo"
> > >    #define MODULE_ALLOC_TAG_VMAP_SIZE	(100000UL * sizeof(struct alloc_tag))
> > >    #define SECTION_START(NAME)		(CODETAG_SECTION_START_PREFIX NAME)
> > > @@ -783,19 +785,6 @@ struct pfn_pool {
> > >    
> > >    #define PFN_POOL_SIZE			((PAGE_SIZE - offsetof(struct pfn_pool, pfns)) / \
> > >    					 sizeof(unsigned long))
> > > -
> > > -/*
> > > - * Skip early PFN recording for a page allocation.  Reuses the
> > > - * %__GFP_NO_OBJ_EXT bit.  Used by __alloc_tag_add_early_pfn() to avoid
> > > - * recursion when allocating pages for the early PFN tracking list
> > > - * itself.
> > > - *
> > > - * Codetags of the pages allocated with __GFP_NO_CODETAG should be
> > > - * cleared (via clear_page_tag_ref()) before freeing the pages to prevent
> > > - * alloc_tag_sub_check() from triggering a warning.
> > > - */
> > > -#define __GFP_NO_CODETAG		__GFP_NO_OBJ_EXT
> > > -
> > >    static struct pfn_pool *current_pfn_pool __initdata;
> > >    
> > >    static void __init __alloc_tag_add_early_pfn(unsigned long pfn)
> > > @@ -806,7 +795,8 @@ static void __init __alloc_tag_add_early_pfn(unsigned long pfn)
> > >    	do {
> > >    		pool = READ_ONCE(current_pfn_pool);
> > >    		if (!pool || atomic_read(&pool->count) >= PFN_POOL_SIZE) {
> > > -			struct page *new_page = alloc_page(__GFP_HIGH | __GFP_NO_CODETAG);
> > > +			struct page *new_page = __alloc_pages(__GFP_HIGH, 0, numa_mem_id(),
> > > +							      NULL, ALLOC_NO_CODETAG);
> > >    			struct pfn_pool *new;
> > >    
> > >    			if (!new_page) {
> > > @@ -837,7 +827,7 @@ typedef void alloc_tag_add_func(unsigned long pfn);
> > >    static alloc_tag_add_func __rcu *alloc_tag_add_early_pfn_ptr __refdata =
> > >    	RCU_INITIALIZER(__alloc_tag_add_early_pfn);
> > >    
> > > -void alloc_tag_add_early_pfn(unsigned long pfn, gfp_t gfp_flags)
> > > +void alloc_tag_add_early_pfn(unsigned long pfn, unsigned int alloc_flags)
> >
> > alloc_tag_add_early_pfn() has three occurrences across the codebase:
> >
> > 1. Definition in mm/alloc_tag.c:830:
> >
> > void alloc_tag_add_early_pfn(unsigned long pfn, unsigned int alloc_flags)
> >
> > 2. Declaration in include/linux/alloc_tag.h:166:
> >
> > void alloc_tag_add_early_pfn(unsigned long pfn, gfp_t gfp_flags)
> >
> > 3. Static inline stub in include/linux/alloc_tag.h:170:
> >
> > static inline void alloc_tag_add_early_pfn(unsigned long pfn, gfp_t
> > gfp_flags) {}
> >
> > This patch updates the definition in alloc_tag.c to take unsigned int
> > alloc_flags,
> >
> > but the two declarations in alloc_tag.h are left with the old gfp_t
> > gfp_flags signature
> >
> > These should be updated to match.
> Yeah ditto, sorry about this and thanks for the review.