Re: [PATCH 0/3] mm: remove page_mapped()
"David Hildenbrand (Arm)" <[email protected]>
| Newsgroups | gmane.linux.ports.sh.devel,gmane.linux.kernel,gmane.linux.kernel.bpf,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <[email protected]> |
On 4/27/26 23:38, Alexei Starovoitov wrote: > On Mon, Apr 27, 2026 at 9:59 PM David Hildenbrand (Arm) > <[email protected]> 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) <[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. > > 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