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

[Brendan Jackman]

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().

> Thanks,
>
> Pankaj
>
>> +		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.