]>
Commit | Line | Data |
---|---|---|
04fd09d4 SL |
1 | From 7a97f0374d5f8245d9e3aeee7e6687a36948bb5a 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 888a9dc13677..506e365cf903 100644 | |
95 | --- a/fs/f2fs/inline.c | |
96 | +++ b/fs/f2fs/inline.c | |
97 | @@ -656,6 +656,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 | @@ -664,7 +670,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 |