projects / thirdparty / kernel / stable-queue.git / blame
| Commit | Line | Data |
|---|---|---|
| ba172962 SL |
1 | From 296ce76228e6dae792fddad8938ee253bc574620 Mon Sep 17 00:00:00 2001 |
| 2 | From: Chao Yu <yuchao0@huawei.com> | |
| 3 | Date: Tue, 12 Mar 2019 15:44:27 +0800 | |
| 4 | Subject: f2fs: fix to avoid deadlock in f2fs_read_inline_dir() | |
| 5 | ||
| 6 | [ Upstream commit aadcef64b22f668c1a107b86d3521d9cac915c24 ] | |
| 7 | ||
| 8 | As Jiqun Li reported in bugzilla: | |
| 9 | ||
| 10 | https://bugzilla.kernel.org/show_bug.cgi?id=202883 | |
| 11 | ||
| 12 | sometimes, dead lock when make system call SYS_getdents64 with fsync() is | |
| 13 | called by another process. | |
| 14 | ||
| 15 | monkey running on android9.0 | |
| 16 | ||
| 17 | 1. task 9785 held sbi->cp_rwsem and waiting lock_page() | |
| 18 | 2. task 10349 held mm_sem and waiting sbi->cp_rwsem | |
| 19 | 3. task 9709 held lock_page() and waiting mm_sem | |
| 20 | ||
| 21 | so this is a dead lock scenario. | |
| 22 | ||
| 23 | task stack is show by crash tools as following | |
| 24 | ||
| 25 | crash_arm64> bt ffffffc03c354080 | |
| 26 | PID: 9785 TASK: ffffffc03c354080 CPU: 1 COMMAND: "RxIoScheduler-3" | |
| 27 | >> #7 [ffffffc01b50fac0] __lock_page at ffffff80081b11e8 | |
| 28 | ||
| 29 | crash-arm64> bt 10349 | |
| 30 | PID: 10349 TASK: ffffffc018b83080 CPU: 1 COMMAND: "BUGLY_ASYNC_UPL" | |
| 31 | >> #3 [ffffffc01f8cfa40] rwsem_down_read_failed at ffffff8008a93afc | |
| 32 | PC: 00000033 LR: 00000000 SP: 00000000 PSTATE: ffffffffffffffff | |
| 33 | ||
| 34 | crash-arm64> bt 9709 | |
| 35 | PID: 9709 TASK: ffffffc03e7f3080 CPU: 1 COMMAND: "IntentService[A" | |
| 36 | >> #3 [ffffffc001e67850] rwsem_down_read_failed at ffffff8008a93afc | |
| 37 | >> #8 [ffffffc001e67b80] el1_ia at ffffff8008084fc4 | |
| 38 | PC: ffffff8008274114 [compat_filldir64+120] | |
| 39 | LR: ffffff80083584d4 [f2fs_fill_dentries+448] | |
| 40 | SP: ffffffc001e67b80 PSTATE: 80400145 | |
| 41 | X29: ffffffc001e67b80 X28: 0000000000000000 X27: 000000000000001a | |
| 42 | X26: 00000000000093d7 X25: ffffffc070d52480 X24: 0000000000000008 | |
| 43 | X23: 0000000000000028 X22: 00000000d43dfd60 X21: ffffffc001e67e90 | |
| 44 | X20: 0000000000000011 X19: ffffff80093a4000 X18: 0000000000000000 | |
| 45 | X17: 0000000000000000 X16: 0000000000000000 X15: 0000000000000000 | |
| 46 | X14: ffffffffffffffff X13: 0000000000000008 X12: 0101010101010101 | |
| 47 | X11: 7f7f7f7f7f7f7f7f X10: 6a6a6a6a6a6a6a6a X9: 7f7f7f7f7f7f7f7f | |
| 48 | X8: 0000000080808000 X7: ffffff800827409c X6: 0000000080808000 | |
| 49 | X5: 0000000000000008 X4: 00000000000093d7 X3: 000000000000001a | |
| 50 | X2: 0000000000000011 X1: ffffffc070d52480 X0: 0000000000800238 | |
| 51 | >> #9 [ffffffc001e67be0] f2fs_fill_dentries at ffffff80083584d0 | |
| 52 | PC: 0000003c LR: 00000000 SP: 00000000 PSTATE: 000000d9 | |
| 53 | X12: f48a02ff X11: d4678960 X10: d43dfc00 X9: d4678ae4 | |
| 54 | X8: 00000058 X7: d4678994 X6: d43de800 X5: 000000d9 | |
| 55 | X4: d43dfc0c X3: d43dfc10 X2: d46799c8 X1: 00000000 | |
| 56 | X0: 00001068 | |
| 57 | ||
| 58 | Below potential deadlock will happen between three threads: | |
| 59 | Thread A Thread B Thread C | |
| 60 | - f2fs_do_sync_file | |
| 61 | - f2fs_write_checkpoint | |
| 62 | - down_write(&sbi->node_change) -- 1) | |
| 63 | - do_page_fault | |
| 64 | - down_write(&mm->mmap_sem) -- 2) | |
| 65 | - do_wp_page | |
| 66 | - f2fs_vm_page_mkwrite | |
| 67 | - getdents64 | |
| 68 | - f2fs_read_inline_dir | |
| 69 | - lock_page -- 3) | |
| 70 | - f2fs_sync_node_pages | |
| 71 | - lock_page -- 3) | |
| 72 | - __do_map_lock | |
| 73 | - down_read(&sbi->node_change) -- 1) | |
| 74 | - f2fs_fill_dentries | |
| 75 | - dir_emit | |
| 76 | - compat_filldir64 | |
| 77 | - do_page_fault | |
| 78 | - down_read(&mm->mmap_sem) -- 2) | |
| 79 | ||
| 80 | Since f2fs_readdir is protected by inode.i_rwsem, there should not be | |
| 81 | any updates in inode page, we're safe to lookup dents in inode page | |
| 82 | without its lock held, so taking off the lock to improve concurrency | |
| 83 | of readdir and avoid potential deadlock. | |
| 84 | ||
| 85 | Reported-by: Jiqun Li <jiqun.li@unisoc.com> | |
| 86 | Signed-off-by: Chao Yu <yuchao0@huawei.com> | |
| 87 | Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> | |
| 88 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
| 89 | --- | |
| 90 | fs/f2fs/inline.c | 8 +++++++- | |
| 91 | 1 file changed, 7 insertions(+), 1 deletion(-) | |
| 92 | ||
| 93 | diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c | |
| 94 | index 115dc219344b..92703efde36e 100644 | |
| 95 | --- a/fs/f2fs/inline.c | |
| 96 | +++ b/fs/f2fs/inline.c | |
| 97 | @@ -661,6 +661,12 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, | |
| 98 | if (IS_ERR(ipage)) | |
| 99 | return PTR_ERR(ipage); | |
| 100 | ||
| 101 | + /* | |
| 102 | + * f2fs_readdir was protected by inode.i_rwsem, it is safe to access | |
| 103 | + * ipage without page's lock held. | |
| 104 | + */ | |
| 105 | + unlock_page(ipage); | |
| 106 | + | |
| 107 | inline_dentry = inline_data_addr(inode, ipage); | |
| 108 | ||
| 109 | make_dentry_ptr_inline(inode, &d, inline_dentry); | |
| 110 | @@ -669,7 +675,7 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, | |
| 111 | if (!err) | |
| 112 | ctx->pos = d.max; | |
| 113 | ||
| 114 | - f2fs_put_page(ipage, 1); | |
| 115 | + f2fs_put_page(ipage, 0); | |
| 116 | return err < 0 ? err : 0; | |
| 117 | } | |
| 118 | ||
| 119 | -- | |
| 120 | 2.19.1 | |
| 121 |