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) <david@kernel.org>
---
David Hildenbrand (Arm) (3):
      sh: use folio_mapped() instead of page_mapped() in sh4_flush_cache_page()
      bpf: arena: use page_ref_count() instead of page_mapped() in arena_free_pages()
      mm: remove page_mapped()

 arch/sh/mm/cache-sh4.c |  2 +-
 include/linux/mm.h     | 10 ----------
 kernel/bpf/arena.c     |  2 +-
 mm/memory.c            |  2 +-
 mm/rmap.c              |  8 ++++----
 5 files changed, 7 insertions(+), 17 deletions(-)

---

base-commit: a2ddbfd1af0f54ea84bf17f0400088815d012e8d

change-id: 20260422-page_mapped-a49a892fa432

--

Cheers,

David

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) <david@kernel.org>
> ---

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

On Mon, Apr 27, 2026 at 9:59 PM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> 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) <david@kernel.org>
> > ---
>
> 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.

Yes, but do NOT. As I explained to Andrew.
It's a git mess. I don't want any more changes that cross trees.

On 4/27/26 23:38, Alexei Starovoitov wrote:
> On Mon, Apr 27, 2026 at 9:59 PM David Hildenbrand (Arm)
> <david@kernel.org> wrote:
>>
>> 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) <david@kernel.org>
>>> ---
>>
>> 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.
> 
> Yes, but do NOT.

In general, do NOT talk to me like that.


As I discussed recently with Ryan, we should update the locking requirements for
zap_vma_range().

I assume friendly BPF folks will take care of fixing this.

-- 
Cheers,

David