Re: [PATCH v2] mm/memory: update stale locking comments for fault handlers

[Aditya Sharma]

Hi David,

Understood. Will send v3 as a new thread, not a reply.

I'll apply the wording changes ("held" and "see FAULT_FLAG_VMA_LOCK") and send out the v3 shortly

Thanks,
Aditya



From: David Hildenbrand (Arm) <david@xxxxxxxxxx>
To: "Aditya Sharma"<adi.sharma@xxxxxxxxxxx>, <linux-mm@xxxxxxxxx>
Cc: <akpm@xxxxxxxxxxxxxxxxxxxx>, <ljs@xxxxxxxxxx>, <Liam.Howlett@xxxxxxxxxx>, <vbabka@xxxxxxxxxx>, <rppt@xxxxxxxxxx>, <surenb@xxxxxxxxxx>, <mhocko@xxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>
Date: Thu, 23 Apr 2026 19:57:21 +0530
Subject: Re: [PATCH v2] mm/memory: update stale locking comments for fault handlers

 > On 4/5/26 19:18, Aditya Sharma wrote: 
 > > Update the comments for wp_page_copy(), do_wp_page(), do_swap_page(), 
 > > do_anonymous_page(), __do_fault(), do_fault(), handle_pte_fault(), 
 > > __handle_mm_fault(), and handle_mm_fault() to concisely clarify that 
 > > they can be entered holding either the mmap_lock or the VMA lock, 
 > > and that the lock may be released upon returning VM_FAULT_RETRY. 
 > > 
 > > Additionally, make the following corrections: 
 > > - In do_anonymous_page(), correct the outdated claim that the function 
 > >   is entered with the PTE "mapped but not yet locked". Since 
 > >   handle_pte_fault() unmaps the empty PTE before routing to 
 > >   do_pte_missing(), the comment now correctly states it is entered 
 > >   with the PTE unmapped and unlocked. 
 > > - In __do_fault(), update the stale reference from __lock_page_retry() 
 > >   to __folio_lock_or_retry(). 
 > > 
 > > Signed-off-by: Aditya Sharma <adi.sharma@xxxxxxxxxxx> 
 >  
 > Next time, please don't send as reply to your previous version 
 >  
 > > --- 
 > > v2: 
 > > - Simplified the comment to concisely state "either the VMA lock or 
 > >   the mmap_lock" instead of a verbose explanation (per David Hildenbrand). 
 > > - Expanded the scope to cover 8 other fault handlers in mm/memory.c 
 > >   that suffered from the same stale mmap_lock comments. 
 > > - Fixed an additional historical inaccuracy in do_anonymous_page() 
 > >   regarding the PTE mapping state on entry. 
 > > - Updated a stale reference in __do_fault() from __lock_page_retry() 
 > >   to __folio_lock_or_retry(). 
 > > 
 > > 
 > >  mm/memory.c | 49 ++++++++++++++++++++++++++----------------------- 
 > >  1 file changed, 26 insertions(+), 23 deletions(-) 
 > > 
 > > diff --git a/mm/memory.c b/mm/memory.c 
 > > index c65e82c86..2b407e3f9 100644 
 > > --- a/mm/memory.c 
 > > +++ b/mm/memory.c 
 > > @@ -3742,8 +3742,8 @@ vm_fault_t __vmf_anon_prepare(struct vm_fault *vmf) 
 > >   * Handle the case of a page which we actually need to copy to a new page, 
 > >   * either due to COW or unsharing. 
 > >   * 
 > > - * Called with mmap_lock locked and the old page referenced, but 
 > > - * without the ptl held. 
 > > + * Called with either the VMA lock or the mmap_lock (FAULT_FLAG_VMA_LOCK 
 > > + * tells you which) 
 >  
 > "Called with either the VMA lock or the mmap_lock *held*" 
 >  
 > Same for other cases. 
 >  
 > Can we shorten "FAULT_FLAG_VMA_LOCK tells you which" to "see FAULT_FLAG_VMA_LOCK" 
 >  
 > ? 
 >  
 > -- 
 > Cheers, 
 >  
 > David 
 >