Re: [PATCH 0/3] mm: remove page_mapped()

On 4/27/26 13:43, David Hildenbrand (Arm) wrote:
> While preparing my slides for an LSF/MM talk, I realized that I did not
> yet remove page_mapped().
> 
> So let's do that. In the BPF arena code it's unclear which memdesc we
> would want to allocate in the future: certainly something with a
> refcount, but likely none with a mapcount. So let's just rely on
> the page refcount instead to decide whether we want to try zapping the
> page from user page tables.
> 
> Signed-off-by: David Hildenbrand (Arm) <[email protected]>
> ---

I scanned AI review and I think it founds something that is not related to this
patch.

We use the page_mapped()->page_ref_count() check as an optimization to avoid
calling zap_vma_range(). We must be able to call it even without that optimization.

Just like the bulk zap call earlier

	if (page_cnt > 1)
		/* bulk zap if multiple pages being freed */
		zap_pages(arena, full_uaddr, page_cnt);

It talks about concurrent "munmap(), unmap_region() executes unmap_vmas()"
racing with our zap_vma_range().

Looking into the details, arena_map_mmap() calls remember_vma(). We reject
mremap and VMA split. arena_vm_close() removes the VMA from the list. The
arena->lock protects our VMA list.

So in zap_pages, the VMA cannot go away. If we find a VMA, ->close could not
have been called yet.

In vma.c, we call remove_vma() after vms_clear_pte(). So after unmapping the
pages and freeing the page tables.

So munmap() can indeed race with zap_vma_range(), and the page_mapped() check
would not have changed anything about that really.


@BPF folks: does BPF take anywhere the mmap lock in read mode before calling
zap_vma_range()? It should do that.

-- 
Cheers,

David