On Thu, Dec 19, 2024 at 10:13:34AM +0100, Peter Zijlstra wrote: > On Wed, Dec 18, 2024 at 01:53:17PM -0800, Suren Baghdasaryan wrote: > > > Ah, ok I see now. I completely misunderstood what for_each_vma_range() > > was doing. > > > > Then I think vma_start_write() should remain inside > > vms_gather_munmap_vmas() and all vmas in mas_detach should be > > No, it must not. You really are not modifying anything yet (except the > split, which we've already noted mark write themselves). > > > write-locked, even the ones we are not modifying. Otherwise what would > > prevent the race I mentioned before? > > > > __mmap_region > > __mmap_prepare > > vms_gather_munmap_vmas // adds vmas to be unmapped into mas_detach, > > // some locked > > by __split_vma(), some not locked > > > > lock_vma_under_rcu() > > vma = mas_walk // finds > > unlocked vma also in mas_detach > > vma_start_read(vma) // > > succeeds since vma is not locked > > // vma->detached, vm_start, > > vm_end checks pass > > // vma is successfully read-locked > > > > vms_clean_up_area(mas_detach) > > vms_clear_ptes > > // steps on a cleared PTE > > So here we have the added complexity that the vma is not unhooked at > all. Is there anything that would prevent a concurrent gup_fast() from > doing the same -- touch a cleared PTE? > > AFAICT two threads, one doing overlapping mmap() and the other doing > gup_fast() can result in exactly this scenario. > > If we don't care about the GUP case, when I'm thinking we should not > care about the lockless RCU case either. Also, at this point we'll just fail to find a page, and that is nothing special. The problem with accessing an unmapped VMA is that the page-table walk will instantiate page-tables. Given this is an overlapping mmap -- we're going to need to those page-tables anyway, so no harm done. Only after the VMA is unlinked must we ensure we don't accidentally re-instantiate page-tables.
