Re: [PATCH v10 12/18] mm: replace vm_lock and detached flag with a reference count
From: Suren Baghdasaryan <surenb@google.com>
Date: 2025-02-20 20:05:55
Also in:
linux-mm, lkml
On Thu, Feb 20, 2025 at 11:03 AM Suren Baghdasaryan [off-list ref] wrote:
On Thu, Feb 20, 2025 at 10:53 AM Heiko Carstens [off-list ref] wrote:quoted
On Thu, Feb 13, 2025 at 02:46:49PM -0800, Suren Baghdasaryan wrote: ...quoted
While this vm_lock replacement does not yet result in a smaller vm_area_struct (it stays at 256 bytes due to cacheline alignment), it allows for further size optimization by structure member regrouping to bring the size of vm_area_struct below 192 bytes. Suggested-by: Peter Zijlstra <peterz@infradead.org> Suggested-by: Matthew Wilcox <willy@infradead.org> Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- Changes since v9 [1]: - Use __refcount_inc_not_zero_limited_acquire() in vma_start_read(), per Hillf Danton - Refactor vma_assert_locked() to avoid vm_refcnt read when CONFIG_DEBUG_VM=n, per Mateusz Guzik - Update changelog, per Wei Yang - Change vma_start_read() to return EAGAIN if vma got isolated and changed lock_vma_under_rcu() back to detect this condition, per Wei Yang - Change VM_BUG_ON_VMA() to WARN_ON_ONCE() when checking vma detached state, per Lorenzo Stoakes - Remove Vlastimil's Reviewed-by since code is changedThis causes crashes (NULL pointer deref) with linux-next when running the ltp test suite; mtest06 (mmap1) test case. The bug seems to be quite obvious:quoted
@@ -6424,15 +6492,18 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm, if (!vma) goto inval; - if (!vma_start_read(vma)) - goto inval; + vma = vma_start_read(vma); + if (IS_ERR_OR_NULL(vma)) {^^^^^^^^^^^^^^^^^^^quoted
+ /* Check if the VMA got isolated after we found it */ + if (PTR_ERR(vma) == -EAGAIN) { + vma_end_read(vma);^^^^^^^^^^^^^^^^Doh! Thanks for reporting! I'll post a fix shortly.
The fix is posted at: https://lore.kernel.org/all/20250220200208.323769-1-surenb@google.com/ (local) Quite embarrassing... I missed removing that extra call. This change was done only in v10, so never appeared before.