Skip to content

Commit 50964c7

Browse files
Wengang-oracleSGCMarkus
authored andcommitted
ocfs2: fix deadlock between setattr and dio_end_io_write
commit 90bd070aae6c4fb5d302f9c4b9c88be60c8197ec upstream. The following deadlock is detected: truncate -> setattr path is waiting for pending direct IO to be done (inode->i_dio_count become zero) with inode->i_rwsem held (down_write). PID: 14827 TASK: ffff881686a9af80 CPU: 20 COMMAND: "ora_p005_hrltd9" #0 __schedule at ffffffff818667cc #1 schedule at ffffffff81866de6 #2 inode_dio_wait at ffffffff812a2d04 #3 ocfs2_setattr at ffffffffc05f322e [ocfs2] #4 notify_change at ffffffff812a5a09 #5 do_truncate at ffffffff812808f5 #6 do_sys_ftruncate.constprop.18 at ffffffff81280cf2 #7 sys_ftruncate at ffffffff81280d8e #8 do_syscall_64 at ffffffff81003949 #9 entry_SYSCALL_64_after_hwframe at ffffffff81a001ad dio completion path is going to complete one direct IO (decrement inode->i_dio_count), but before that it hung at locking inode->i_rwsem: #0 __schedule+700 at ffffffff818667cc #1 schedule+54 at ffffffff81866de6 #2 rwsem_down_write_failed+536 at ffffffff8186aa28 #3 call_rwsem_down_write_failed+23 at ffffffff8185a1b7 #4 down_write+45 at ffffffff81869c9d #5 ocfs2_dio_end_io_write+180 at ffffffffc05d5444 [ocfs2] #6 ocfs2_dio_end_io+85 at ffffffffc05d5a85 [ocfs2] #7 dio_complete+140 at ffffffff812c873c #8 dio_aio_complete_work+25 at ffffffff812c89f9 #9 process_one_work+361 at ffffffff810b1889 #10 worker_thread+77 at ffffffff810b233d #11 kthread+261 at ffffffff810b7fd5 #12 ret_from_fork+62 at ffffffff81a0035e Thus above forms ABBA deadlock. The same deadlock was mentioned in upstream commit 28f5a8a7c033 ("ocfs2: should wait dio before inode lock in ocfs2_setattr()"). It seems that that commit only removed the cluster lock (the victim of above dead lock) from the ABBA deadlock party. End-user visible effects: Process hang in truncate -> ocfs2_setattr path and other processes hang at ocfs2_dio_end_io_write path. This is to fix the deadlock itself. It removes inode_lock() call from dio completion path to remove the deadlock and add ip_alloc_sem lock in setattr path to synchronize the inode modifications. [wen.gang.wang@oracle.com: remove the "had_alloc_lock" as suggested] Link: https://lkml.kernel.org/r/20210402171344.1605-1-wen.gang.wang@oracle.com Link: https://lkml.kernel.org/r/20210331203654.3911-1-wen.gang.wang@oracle.com Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> Cc: Mark Fasheh <mark@fasheh.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Junxiao Bi <junxiao.bi@oracle.com> Cc: Changwei Ge <gechangwei@live.cn> Cc: Gang He <ghe@suse.com> Cc: Jun Piao <piaojun@huawei.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 942c0ce commit 50964c7

2 files changed

Lines changed: 7 additions & 12 deletions

File tree

fs/ocfs2/aops.c

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2309,7 +2309,7 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
23092309
struct ocfs2_alloc_context *meta_ac = NULL;
23102310
handle_t *handle = NULL;
23112311
loff_t end = offset + bytes;
2312-
int ret = 0, credits = 0, locked = 0;
2312+
int ret = 0, credits = 0;
23132313

23142314
ocfs2_init_dealloc_ctxt(&dealloc);
23152315

@@ -2320,13 +2320,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
23202320
!dwc->dw_orphaned)
23212321
goto out;
23222322

2323-
/* ocfs2_file_write_iter will get i_mutex, so we need not lock if we
2324-
* are in that context. */
2325-
if (dwc->dw_writer_pid != task_pid_nr(current)) {
2326-
inode_lock(inode);
2327-
locked = 1;
2328-
}
2329-
23302323
ret = ocfs2_inode_lock(inode, &di_bh, 1);
23312324
if (ret < 0) {
23322325
mlog_errno(ret);
@@ -2401,8 +2394,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
24012394
if (meta_ac)
24022395
ocfs2_free_alloc_context(meta_ac);
24032396
ocfs2_run_deallocs(osb, &dealloc);
2404-
if (locked)
2405-
inode_unlock(inode);
24062397
ocfs2_dio_free_write_ctx(inode, dwc);
24072398

24082399
return ret;

fs/ocfs2/file.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,22 +1250,24 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
12501250
goto bail_unlock;
12511251
}
12521252
}
1253+
down_write(&OCFS2_I(inode)->ip_alloc_sem);
12531254
handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS +
12541255
2 * ocfs2_quota_trans_credits(sb));
12551256
if (IS_ERR(handle)) {
12561257
status = PTR_ERR(handle);
12571258
mlog_errno(status);
1258-
goto bail_unlock;
1259+
goto bail_unlock_alloc;
12591260
}
12601261
status = __dquot_transfer(inode, transfer_to);
12611262
if (status < 0)
12621263
goto bail_commit;
12631264
} else {
1265+
down_write(&OCFS2_I(inode)->ip_alloc_sem);
12641266
handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS);
12651267
if (IS_ERR(handle)) {
12661268
status = PTR_ERR(handle);
12671269
mlog_errno(status);
1268-
goto bail_unlock;
1270+
goto bail_unlock_alloc;
12691271
}
12701272
}
12711273

@@ -1278,6 +1280,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
12781280

12791281
bail_commit:
12801282
ocfs2_commit_trans(osb, handle);
1283+
bail_unlock_alloc:
1284+
up_write(&OCFS2_I(inode)->ip_alloc_sem);
12811285
bail_unlock:
12821286
if (status && inode_locked) {
12831287
ocfs2_inode_unlock_tracker(inode, 1, &oh, had_lock);

0 commit comments

Comments
 (0)