Skip to content

Commit 5dc21f0

Browse files
weiny2akpm00
authored andcommitted
mm/shmem: ensure proper fallback if page faults
The kernel test robot flagged a recursive lock as a result of a conversion from kmap_atomic() to kmap_local_folio()[Link] The cause was due to the code depending on the kmap_atomic() side effect of disabling page faults. In that case the code expects the fault to fail and take the fallback case. git archaeology implied that the recursion may not be an actual bug.[1] However, depending on the implementation of the mmap_lock and the condition of the call there may still be a deadlock.[2] So this is not purely a lockdep issue. Considering a single threaded call stack there are 3 options. 1) Different mm's are in play (no issue) 2) Readlock implementation is recursive and same mm is in play (no issue) 3) Readlock implementation is _not_ recursive (issue) The mmap_lock is recursive so with a single thread there is no issue. However, Matthew pointed out a deadlock scenario when you consider additional process' and threads thusly. "The readlock implementation is only recursive if nobody else has taken a write lock. If you have a multithreaded process, one of the other threads can call mmap() and that will prevent recursion (due to fairness). Even if it's a different process that you're trying to acquire the mmap read lock on, you can still get into a deadly embrace. eg: process A thread 1 takes read lock on own mmap_lock process A thread 2 calls mmap, blocks taking write lock process B thread 1 takes page fault, read lock on own mmap lock process B thread 2 calls mmap, blocks taking write lock process A thread 1 blocks taking read lock on process B process B thread 1 blocks taking read lock on process A Now all four threads are blocked waiting for each other." Regardless using pagefault_disable() ensures that no matter what locking implementation is used a deadlock will not occur. Add an explicit pagefault_disable() and a big comment to explain this for future souls looking at this code. [1] https://lore.kernel.org/all/Y1MymJ%2FINb45AdaY@iweiny-desk3/ [2] https://lore.kernel.org/lkml/Y1bXBtGTCym77%[email protected]/ Link: https://lkml.kernel.org/r/[email protected] Link: https://lore.kernel.org/r/[email protected] Fixes: 7a7256d ("shmem: convert shmem_mfill_atomic_pte() to use a folio") Signed-off-by: Ira Weiny <[email protected]> Reported-by: Matthew Wilcox (Oracle) <[email protected]> Reported-by: kernel test robot <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Peter Xu <[email protected]> Cc: Andrea Arcangeli <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 5521de7 commit 5dc21f0

File tree

1 file changed

+17
-0
lines changed

1 file changed

+17
-0
lines changed

mm/shmem.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,9 +2424,26 @@ int shmem_mfill_atomic_pte(struct mm_struct *dst_mm,
24242424

24252425
if (!zeropage) { /* COPY */
24262426
page_kaddr = kmap_local_folio(folio, 0);
2427+
/*
2428+
* The read mmap_lock is held here. Despite the
2429+
* mmap_lock being read recursive a deadlock is still
2430+
* possible if a writer has taken a lock. For example:
2431+
*
2432+
* process A thread 1 takes read lock on own mmap_lock
2433+
* process A thread 2 calls mmap, blocks taking write lock
2434+
* process B thread 1 takes page fault, read lock on own mmap lock
2435+
* process B thread 2 calls mmap, blocks taking write lock
2436+
* process A thread 1 blocks taking read lock on process B
2437+
* process B thread 1 blocks taking read lock on process A
2438+
*
2439+
* Disable page faults to prevent potential deadlock
2440+
* and retry the copy outside the mmap_lock.
2441+
*/
2442+
pagefault_disable();
24272443
ret = copy_from_user(page_kaddr,
24282444
(const void __user *)src_addr,
24292445
PAGE_SIZE);
2446+
pagefault_enable();
24302447
kunmap_local(page_kaddr);
24312448

24322449
/* fallback to copy_from_user outside mmap_lock */

0 commit comments

Comments
 (0)