* [PATCH AUTOSEL 4.14 003/123] f2fs: fix to avoid deadlock in f2fs_read_inline_dir()
[not found] <20190327181628.15899-1-sashal@kernel.org>
@ 2019-03-27 18:14 ` Sasha Levin
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 021/123] f2fs: do not use mutex lock in atomic context Sasha Levin
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 022/123] f2fs: fix to data block override node segment by mistake Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2019-03-27 18:14 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Jaegeuk Kim, linux-f2fs-devel
From: Chao Yu <yuchao0@huawei.com>
[ Upstream commit aadcef64b22f668c1a107b86d3521d9cac915c24 ]
As Jiqun Li reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=202883
sometimes, dead lock when make system call SYS_getdents64 with fsync() is
called by another process.
monkey running on android9.0
1. task 9785 held sbi->cp_rwsem and waiting lock_page()
2. task 10349 held mm_sem and waiting sbi->cp_rwsem
3. task 9709 held lock_page() and waiting mm_sem
so this is a dead lock scenario.
task stack is show by crash tools as following
crash_arm64> bt ffffffc03c354080
PID: 9785 TASK: ffffffc03c354080 CPU: 1 COMMAND: "RxIoScheduler-3"
>> #7 [ffffffc01b50fac0] __lock_page at ffffff80081b11e8
crash-arm64> bt 10349
PID: 10349 TASK: ffffffc018b83080 CPU: 1 COMMAND: "BUGLY_ASYNC_UPL"
>> #3 [ffffffc01f8cfa40] rwsem_down_read_failed at ffffff8008a93afc
PC: 00000033 LR: 00000000 SP: 00000000 PSTATE: ffffffffffffffff
crash-arm64> bt 9709
PID: 9709 TASK: ffffffc03e7f3080 CPU: 1 COMMAND: "IntentService[A"
>> #3 [ffffffc001e67850] rwsem_down_read_failed at ffffff8008a93afc
>> #8 [ffffffc001e67b80] el1_ia at ffffff8008084fc4
PC: ffffff8008274114 [compat_filldir64+120]
LR: ffffff80083584d4 [f2fs_fill_dentries+448]
SP: ffffffc001e67b80 PSTATE: 80400145
X29: ffffffc001e67b80 X28: 0000000000000000 X27: 000000000000001a
X26: 00000000000093d7 X25: ffffffc070d52480 X24: 0000000000000008
X23: 0000000000000028 X22: 00000000d43dfd60 X21: ffffffc001e67e90
X20: 0000000000000011 X19: ffffff80093a4000 X18: 0000000000000000
X17: 0000000000000000 X16: 0000000000000000 X15: 0000000000000000
X14: ffffffffffffffff X13: 0000000000000008 X12: 0101010101010101
X11: 7f7f7f7f7f7f7f7f X10: 6a6a6a6a6a6a6a6a X9: 7f7f7f7f7f7f7f7f
X8: 0000000080808000 X7: ffffff800827409c X6: 0000000080808000
X5: 0000000000000008 X4: 00000000000093d7 X3: 000000000000001a
X2: 0000000000000011 X1: ffffffc070d52480 X0: 0000000000800238
>> #9 [ffffffc001e67be0] f2fs_fill_dentries at ffffff80083584d0
PC: 0000003c LR: 00000000 SP: 00000000 PSTATE: 000000d9
X12: f48a02ff X11: d4678960 X10: d43dfc00 X9: d4678ae4
X8: 00000058 X7: d4678994 X6: d43de800 X5: 000000d9
X4: d43dfc0c X3: d43dfc10 X2: d46799c8 X1: 00000000
X0: 00001068
Below potential deadlock will happen between three threads:
Thread A Thread B Thread C
- f2fs_do_sync_file
- f2fs_write_checkpoint
- down_write(&sbi->node_change) -- 1)
- do_page_fault
- down_write(&mm->mmap_sem) -- 2)
- do_wp_page
- f2fs_vm_page_mkwrite
- getdents64
- f2fs_read_inline_dir
- lock_page -- 3)
- f2fs_sync_node_pages
- lock_page -- 3)
- __do_map_lock
- down_read(&sbi->node_change) -- 1)
- f2fs_fill_dentries
- dir_emit
- compat_filldir64
- do_page_fault
- down_read(&mm->mmap_sem) -- 2)
Since f2fs_readdir is protected by inode.i_rwsem, there should not be
any updates in inode page, we're safe to lookup dents in inode page
without its lock held, so taking off the lock to improve concurrency
of readdir and avoid potential deadlock.
Reported-by: Jiqun Li <jiqun.li@unisoc.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/f2fs/inline.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 888a9dc13677..506e365cf903 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -656,6 +656,12 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx,
if (IS_ERR(ipage))
return PTR_ERR(ipage);
+ /*
+ * f2fs_readdir was protected by inode.i_rwsem, it is safe to access
+ * ipage without page's lock held.
+ */
+ unlock_page(ipage);
+
inline_dentry = inline_data_addr(inode, ipage);
make_dentry_ptr_inline(inode, &d, inline_dentry);
@@ -664,7 +670,7 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx,
if (!err)
ctx->pos = d.max;
- f2fs_put_page(ipage, 1);
+ f2fs_put_page(ipage, 0);
return err < 0 ? err : 0;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH AUTOSEL 4.14 021/123] f2fs: do not use mutex lock in atomic context
[not found] <20190327181628.15899-1-sashal@kernel.org>
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 003/123] f2fs: fix to avoid deadlock in f2fs_read_inline_dir() Sasha Levin
@ 2019-03-27 18:14 ` Sasha Levin
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 022/123] f2fs: fix to data block override node segment by mistake Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2019-03-27 18:14 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Jaegeuk Kim, linux-f2fs-devel
From: Sahitya Tummala <stummala@codeaurora.org>
[ Upstream commit 9083977dabf3833298ddcd40dee28687f1e6b483 ]
Fix below warning coming because of using mutex lock in atomic context.
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:98
in_atomic(): 1, irqs_disabled(): 0, pid: 585, name: sh
Preemption disabled at: __radix_tree_preload+0x28/0x130
Call trace:
dump_backtrace+0x0/0x2b4
show_stack+0x20/0x28
dump_stack+0xa8/0xe0
___might_sleep+0x144/0x194
__might_sleep+0x58/0x8c
mutex_lock+0x2c/0x48
f2fs_trace_pid+0x88/0x14c
f2fs_set_node_page_dirty+0xd0/0x184
Do not use f2fs_radix_tree_insert() to avoid doing cond_resched() with
spin_lock() acquired.
Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/f2fs/trace.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/trace.c b/fs/f2fs/trace.c
index bccbbf2616d2..8ac1851a21c0 100644
--- a/fs/f2fs/trace.c
+++ b/fs/f2fs/trace.c
@@ -61,6 +61,7 @@ void f2fs_trace_pid(struct page *page)
set_page_private(page, (unsigned long)pid);
+retry:
if (radix_tree_preload(GFP_NOFS))
return;
@@ -71,7 +72,12 @@ void f2fs_trace_pid(struct page *page)
if (p)
radix_tree_delete(&pids, pid);
- f2fs_radix_tree_insert(&pids, pid, current);
+ if (radix_tree_insert(&pids, pid, current)) {
+ spin_unlock(&pids_lock);
+ radix_tree_preload_end();
+ cond_resched();
+ goto retry;
+ }
trace_printk("%3x:%3x %4x %-16s\n",
MAJOR(inode->i_sb->s_dev), MINOR(inode->i_sb->s_dev),
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH AUTOSEL 4.14 022/123] f2fs: fix to data block override node segment by mistake
[not found] <20190327181628.15899-1-sashal@kernel.org>
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 003/123] f2fs: fix to avoid deadlock in f2fs_read_inline_dir() Sasha Levin
2019-03-27 18:14 ` [PATCH AUTOSEL 4.14 021/123] f2fs: do not use mutex lock in atomic context Sasha Levin
@ 2019-03-27 18:14 ` Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2019-03-27 18:14 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Jaegeuk Kim, linux-f2fs-devel
From: zhengliang <zhengliang6@huawei.com>
[ Upstream commit a0770e13c8da83bdb64738c0209ab02dd3cfff8b ]
v4: Rearrange the previous three versions.
The following scenario could lead to data block override by mistake.
TASK A | TASK kworker | TASK B | TASK C
| | |
open | | |
write | | |
close | | |
| f2fs_write_data_pages | |
| f2fs_write_cache_pages | |
| f2fs_outplace_write_data | |
| f2fs_allocate_data_block (get block in seg S, | |
| S is full, and only | |
| have this valid data | |
| block) | |
| allocate_segment | |
| locate_dirty_segment (mark S as PRE) | |
| f2fs_submit_page_write (submit but is not | |
| written on dev) | |
unlink | | |
iput_final | | |
f2fs_drop_inode | | |
f2fs_truncate | | |
(not evict) | | |
| | write_checkpoint |
| | flush merged bio but not wait file data writeback |
| | set_prefree_as_free (mark S as FREE) |
| | | update NODE/DATA
| | | allocate_segment (select S)
| writeback done | |
So we need to guarantee io complete before truncate inode in f2fs_drop_inode.
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/f2fs/super.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fc5c41257e68..6c61badb07fb 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -668,6 +668,10 @@ static int f2fs_drop_inode(struct inode *inode)
sb_start_intwrite(inode->i_sb);
f2fs_i_size_write(inode, 0);
+ f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
+ inode, NULL, 0, DATA);
+ truncate_inode_pages_final(inode->i_mapping);
+
if (F2FS_HAS_BLOCKS(inode))
f2fs_truncate(inode);
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread