On Wed, Jul 9, 2025 at 3:03 PM Vlastimil Babka <vba...@suse.cz> wrote: > > On 7/9/25 16:43, Suren Baghdasaryan wrote: > > On Wed, Jul 9, 2025 at 1:57 AM Vlastimil Babka <vba...@suse.cz> wrote: > >> > >> On 7/8/25 01:10, Suren Baghdasaryan wrote: > >> >>> + rcu_read_unlock(); > >> >>> + vma = lock_vma_under_mmap_lock(mm, iter, address); > >> >>> + rcu_read_lock(); > >> >> OK I guess we hold the RCU lock the whole time as we traverse except > >> >> when > >> >> we lock under mmap lock. > >> > Correct. > >> > >> I wonder if it's really necessary? Can't it be done just inside > >> lock_next_vma()? It would also avoid the unlock/lock dance quoted above. > >> > >> Even if we later manage to extend this approach to smaps and employ rcu > >> locking to traverse the page tables, I'd think it's best to separate and > >> fine-grain the rcu lock usage for vma iterator and page tables, if only to > >> avoid too long time under the lock. > > > > I thought we would need to be in the same rcu read section while > > traversing the maple tree using vma_next() but now looking at it, > > maybe we can indeed enter only while finding and locking the next > > vma... > > Liam, would that work? I see struct ma_state containing a node field. > > Can it be freed from under us if we find a vma, exit rcu read section > > then re-enter rcu and use the same iterator to find the next vma? > > If the rcu protection needs to be contigous, and patch 8 avoids the issue by > always doing vma_iter_init() after rcu_read_lock() (but does it really avoid > the issue or is it why we see the syzbot reports?) then I guess in the code > quoted above we also need a vma_iter_init() after the rcu_read_lock(), > because although the iterator was used briefly under mmap_lock protection, > that was then unlocked and there can be a race before the rcu_read_lock().
Quite true. So, let's wait for Liam's confirmation and based on his answer I'll change the patch by either reducing the rcu read section or adding the missing vma_iter_init() after we switch to mmap_lock.
