Re: [PATCH v2 20/22] mm/page_alloc: implement __GFP_UNMAPPED|__GFP_ZERO allocations

[Gupta, Pankaj]

Hi Brendan,

> Hi Pankaj,
>
> Thanks for taking a look!
>
> On Wed Jun 17, 2026 at 11:32 AM UTC, Pankaj Gupta wrote:
> ...
> > > +static inline void clear_page_mermap(struct page *page, unsigned int numpages)
> > > +{
> > > +	void *mermap;
> > > +
> > > +	BUILD_BUG_ON(IS_ENABLED(CONFIG_HIGHMEM));
> > > +
> > I was thinking — would the fast path read better if we moved
> > migrate_disable()/migrate_enable() up a level, into clear_page_mermap()
> > itself? i.e
> >
> > migrate_disable() -> here
> >
> > > +	/* Fast path: single mapping (may fail under preemption). */
> > > +	mermap = mermap_get(page, numpages << PAGE_SHIFT, PAGE_KERNEL_NOGLOBAL);
> > > +	if (mermap) {
> > > +		void *buf = kasan_reset_tag(mermap_addr(mermap));
> > > +
> > > +		for (int i = 0; i < numpages; i++)
> > > +			clear_page(buf + (i << PAGE_SHIFT));
> > > +		mermap_put(mermap);
> > migrate_enable() -> here
> >
> > Right now we split the migrate_disable/enable() a layer below across
> > mermap_get() and mermap_put().
> >
> > Would it make sense to embed that in the clear_page_mermap() API itself
> > — mirroring how we already disable IRQs at this level for the
> > mermap_get_reserved()?
> Hm, I guess the symmetry would make sense. The reason it's this way is
> because the mermap_{get,put}() API mirrors kmap_[un]local_page() etc
> which do the migrate_disable() internally while there is no precedence
> (AFAIK) for an API that automatically {dis,en}ables IRQs. That might
> also be a side-effect of the fact that local_irq_save() requires a local
> variable while migrate_disble() stores its state globally. Similarly,
> there is no equivalent of lockdep_assert_preemption_disabled().

Makes sense. There is cant_migrate() though, maybe in current code we 
could use in mermap_put()?

> > > +		return;
> > > +	}
> > > +
> > > +	/* Slow path, map each page individually (always succeeds). */
> > > +	for (int i = 0; i < numpages; i++) {
> > > +		unsigned long flags;
> > > +
> > > +		local_irq_save(flags);
> > > +		mermap = mermap_get_reserved(page + i, PAGE_KERNEL_NOGLOBAL);
> > > +		clear_page(kasan_reset_tag(mermap_addr(mermap)));
> > > +		mermap_put(mermap);
> > > +		local_irq_restore(flags);
> BTW, writing the above just made me realise: we don't need to disable
> IRQs here, only preemption.
>
> In a WIP version I did have mermap_get_reserved() requiring irqsave, so
> that you could theoretically use it from an IRQ. But actually I don't
> think that makes any sense since it depends on state in current->mm.

Agree.

Thanks,

Pankaj

>