Skip to content

Commit

Permalink
ocfs2: fix deadlock between setattr and dio_end_io_write
Browse files Browse the repository at this point in the history
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 [ffffc9000bcf3c08] __schedule at ffffffff818667cc
#1 [ffffc9000bcf3ca0] schedule at ffffffff81866de6
#2 [ffffc9000bcf3cb8] inode_dio_wait at ffffffff812a2d04
#3 [ffffc9000bcf3d28] ocfs2_setattr at ffffffffc05f322e [ocfs2]
#4 [ffffc9000bcf3e18] notify_change at ffffffff812a5a09
#5 [ffffc9000bcf3e60] do_truncate at ffffffff812808f5
#6 [ffffc9000bcf3ed8] do_sys_ftruncate.constprop.18 at ffffffff81280cf2
#7 [ffffc9000bcf3f18] sys_ftruncate at ffffffff81280d8e
#8 [ffffc9000bcf3f28] do_syscall_64 at ffffffff81003949
#9 [ffffc9000bcf3f50] 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 hang at locking inode->i_rwsem.

 #0 [ffffc90009b47b40] __schedule+700 at ffffffff818667cc
 #1 [ffffc90009b47bd8] schedule+54 at ffffffff81866de6
 #2 [ffffc90009b47bf0] rwsem_down_write_failed+536 at ffffffff8186aa28
 #3 [ffffc90009b47c88] call_rwsem_down_write_failed+23 at ffffffff8185a1b7
 #4 [ffffc90009b47cd0] down_write+45 at ffffffff81869c9d
 #5 [ffffc90009b47ce8] ocfs2_dio_end_io_write+180 at ffffffffc05d5444 [ocfs2]
 #6 [ffffc90009b47dd8] ocfs2_dio_end_io+85 at ffffffffc05d5a85 [ocfs2]
 #7 [ffffc90009b47e18] dio_complete+140 at ffffffff812c873c
 #8 [ffffc90009b47e50] dio_aio_complete_work+25 at ffffffff812c89f9
 #9 [ffffc90009b47e60] process_one_work+361 at ffffffff810b1889
#10 [ffffc90009b47ea8] worker_thread+77 at ffffffff810b233d
#11 [ffffc90009b47f08] kthread+261 at ffffffff810b7fd5
#12 [ffffc90009b47f50] ret_from_fork+62 at ffffffff81a0035e

Thus above forms ABBA deadlock. The same deadlock was mentioned in
upstream commit 28f5a8a. well, it seems
that that commit just removed 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 its self. 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.

Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Wengang Wang <[email protected]>
Cc: Mark Fasheh <[email protected]>
Cc: Joel Becker <[email protected]>
Cc: Junxiao Bi <[email protected]>
Cc: Joseph Qi <[email protected]>
Cc: Changwei Ge <[email protected]>
Cc: Gang He <[email protected]>
Cc: Jun Piao <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Stephen Rothwell <[email protected]>
  • Loading branch information
Wengang-oracle authored and sfrothwell committed Apr 1, 2021
1 parent 969f31e commit 685c64f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
11 changes: 1 addition & 10 deletions fs/ocfs2/aops.c
Original file line number Diff line number Diff line change
Expand Up @@ -2295,7 +2295,7 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
struct ocfs2_alloc_context *meta_ac = NULL;
handle_t *handle = NULL;
loff_t end = offset + bytes;
int ret = 0, credits = 0, locked = 0;
int ret = 0, credits = 0;

ocfs2_init_dealloc_ctxt(&dealloc);

Expand All @@ -2306,13 +2306,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
!dwc->dw_orphaned)
goto out;

/* ocfs2_file_write_iter will get i_mutex, so we need not lock if we
* are in that context. */
if (dwc->dw_writer_pid != task_pid_nr(current)) {
inode_lock(inode);
locked = 1;
}

ret = ocfs2_inode_lock(inode, &di_bh, 1);
if (ret < 0) {
mlog_errno(ret);
Expand Down Expand Up @@ -2393,8 +2386,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
if (meta_ac)
ocfs2_free_alloc_context(meta_ac);
ocfs2_run_deallocs(osb, &dealloc);
if (locked)
inode_unlock(inode);
ocfs2_dio_free_write_ctx(inode, dwc);

return ret;
Expand Down
11 changes: 10 additions & 1 deletion fs/ocfs2/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ int ocfs2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
handle_t *handle = NULL;
struct dquot *transfer_to[MAXQUOTAS] = { };
int qtype;
int had_lock;
int had_lock, had_alloc_lock = 0;
struct ocfs2_lock_holder oh;

trace_ocfs2_setattr(inode, dentry,
Expand Down Expand Up @@ -1245,6 +1245,9 @@ int ocfs2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
goto bail_unlock;
}
}
down_write(&OCFS2_I(inode)->ip_alloc_sem);
had_alloc_lock = 1;

handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS +
2 * ocfs2_quota_trans_credits(sb));
if (IS_ERR(handle)) {
Expand All @@ -1256,6 +1259,9 @@ int ocfs2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
if (status < 0)
goto bail_commit;
} else {
down_write(&OCFS2_I(inode)->ip_alloc_sem);
had_alloc_lock = 1;

handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS);
if (IS_ERR(handle)) {
status = PTR_ERR(handle);
Expand All @@ -1274,6 +1280,9 @@ int ocfs2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
bail_commit:
ocfs2_commit_trans(osb, handle);
bail_unlock:
if (had_alloc_lock)
up_write(&OCFS2_I(inode)->ip_alloc_sem);

if (status && inode_locked) {
ocfs2_inode_unlock_tracker(inode, 1, &oh, had_lock);
inode_locked = 0;
Expand Down

0 comments on commit 685c64f

Please sign in to comment.