On Wed, Jul 9, 2025 at 3:03 PM Vlastimil Babka vbabka@suse.cz wrote:
On 7/9/25 16:43, Suren Baghdasaryan wrote:
On Wed, Jul 9, 2025 at 1:57 AM Vlastimil Babka vbabka@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.