mm: have mmap_action_complete() handle the rmap lock and unmap

Rather than have the callers handle this both the rmap lock release and
unmapping the VMA on error, handle it within the mmap_action_complete()
logic where it makes sense to, being careful not to unlock twice.

This simplifies the logic and makes it harder to make mistake with this,
while retaining correct behaviour with regard to avoiding deadlocks.

Also replace the call_action_complete() function with a direct invocation
of mmap_action_complete() as the abstraction is no longer required.

Also update the VMA tests to reflect this change.

Link: https://lkml.kernel.org/r/8d1ee8ebd3542d006a47e8382fb80cf5b57ecf10.1774045440.git.ljs@kernel.org
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bodo Stroesser <bostroesser@gmail.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Clemens Ladisch <clemens@ladisch.de>
Cc: David Hildenbrand <david@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Long Li <longli@microsoft.com>
Cc: Marc Dionne <marc.dionne@auristor.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Pedro Falcato <pfalcato@suse.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

authored by

Lorenzo Stoakes (Oracle) and committed by

Andrew Morton 2 months ago 382c0f28 04501e75

+40 -54

4 changed files

expand all

internal.h

util.c

vma.c

tools

testing

vma

include

dup.h

+19

mm/internal.h

··· 1863 1863 return 0; 1864 1864 } 1865 1865 1866 + /* 1867 + * When we succeed an mmap action or just before we unmap a VMA on error, we 1868 + * need to ensure any rmap lock held is released. On unmap it's required to 1869 + * avoid a deadlock. 1870 + */ 1871 + static inline void maybe_rmap_unlock_action(struct vm_area_struct *vma, 1872 + struct mmap_action *action) 1873 + { 1874 + struct file *file; 1875 + 1876 + if (!action->hide_from_rmap_until_complete) 1877 + return; 1878 + 1879 + VM_WARN_ON_ONCE(vma_is_anonymous(vma)); 1880 + file = vma->vm_file; 1881 + i_mmap_unlock_write(file->f_mapping); 1882 + action->hide_from_rmap_until_complete = false; 1883 + } 1884 + 1866 1885 #ifdef CONFIG_MMU_NOTIFIER 1867 1886 static inline bool clear_flush_young_ptes_notify(struct vm_area_struct *vma, 1868 1887 unsigned long addr, pte_t *ptep, unsigned int nr)

+19 -22

mm/util.c

··· 1219 1219 action->hide_from_rmap_until_complete = false; 1220 1220 1221 1221 set_vma_from_desc(vma, &desc); 1222 - err = mmap_action_complete(vma, action); 1223 - if (err) { 1224 - const size_t len = vma_pages(vma) << PAGE_SHIFT; 1225 - 1226 - do_munmap(current->mm, vma->vm_start, len, NULL); 1227 - } 1228 - return err; 1222 + return mmap_action_complete(vma, action); 1229 1223 } 1230 1224 EXPORT_SYMBOL(compat_vma_mmap); 1231 1225 ··· 1314 1320 static int mmap_action_finish(struct vm_area_struct *vma, 1315 1321 struct mmap_action *action, int err) 1316 1322 { 1323 + size_t len; 1324 + 1325 + if (!err && action->success_hook) 1326 + err = action->success_hook(vma); 1327 + 1328 + /* do_munmap() might take rmap lock, so release if held. */ 1329 + maybe_rmap_unlock_action(vma, action); 1330 + if (!err) 1331 + return 0; 1332 + 1317 1333 /* 1318 1334 * If an error occurs, unmap the VMA altogether and return an error. We 1319 1335 * only clear the newly allocated VMA, since this function is only 1320 1336 * invoked if we do NOT merge, so we only clean up the VMA we created. 1321 1337 */ 1322 - if (err) { 1323 - if (action->error_hook) { 1324 - /* We may want to filter the error. */ 1325 - err = action->error_hook(err); 1326 - 1327 - /* The caller should not clear the error. */ 1328 - VM_WARN_ON_ONCE(!err); 1329 - } 1330 - return err; 1338 + len = vma_pages(vma) << PAGE_SHIFT; 1339 + do_munmap(current->mm, vma->vm_start, len, NULL); 1340 + if (action->error_hook) { 1341 + /* We may want to filter the error. */ 1342 + err = action->error_hook(err); 1343 + /* The caller should not clear the error. */ 1344 + VM_WARN_ON_ONCE(!err); 1331 1345 } 1332 - 1333 - if (action->success_hook) 1334 - return action->success_hook(vma); 1335 - 1336 - return 0; 1346 + return err; 1337 1347 } 1338 1348 1339 1349 #ifdef CONFIG_MMU ··· 1375 1377 */ 1376 1378 int mmap_action_complete(struct vm_area_struct *vma, 1377 1379 struct mmap_action *action) 1378 - 1379 1380 { 1380 1381 int err = 0; 1381 1382

+1 -25

mm/vma.c

··· 2729 2729 return false; 2730 2730 } 2731 2731 2732 - static int call_action_complete(struct mmap_state *map, 2733 - struct mmap_action *action, 2734 - struct vm_area_struct *vma) 2735 - { 2736 - int err; 2737 - 2738 - err = mmap_action_complete(vma, action); 2739 - 2740 - /* If we held the file rmap we need to release it. */ 2741 - if (action->hide_from_rmap_until_complete) { 2742 - struct file *file = vma->vm_file; 2743 - 2744 - i_mmap_unlock_write(file->f_mapping); 2745 - } 2746 - 2747 - if (err) { 2748 - const size_t len = vma_pages(vma) << PAGE_SHIFT; 2749 - 2750 - do_munmap(current->mm, vma->vm_start, len, NULL); 2751 - } 2752 - 2753 - return err; 2754 - } 2755 - 2756 2732 static unsigned long __mmap_region(struct file *file, unsigned long addr, 2757 2733 unsigned long len, vma_flags_t vma_flags, 2758 2734 unsigned long pgoff, struct list_head *uf) ··· 2780 2804 __mmap_complete(&map, vma); 2781 2805 2782 2806 if (have_mmap_prepare && allocated_new) { 2783 - error = call_action_complete(&map, &desc.action, vma); 2807 + error = mmap_action_complete(vma, &desc.action); 2784 2808 2785 2809 if (error) 2786 2810 return error;

+1 -7

tools/testing/vma/include/dup.h

··· 1300 1300 action->hide_from_rmap_until_complete = false; 1301 1301 1302 1302 set_vma_from_desc(vma, &desc); 1303 - err = mmap_action_complete(vma, action); 1304 - if (err) { 1305 - const size_t len = vma_pages(vma) << PAGE_SHIFT; 1306 - 1307 - do_munmap(current->mm, vma->vm_start, len, NULL); 1308 - } 1309 - return err; 1303 + return mmap_action_complete(vma, action); 1310 1304 } 1311 1305 1312 1306 static inline void vma_iter_init(struct vma_iterator *vmi,

Configure Feed

Configure Feed