Message ID | 59F81636.70508@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, 31 Oct 2017 14:20:38 +0800 alex chen <alex.chen@huawei.com> wrote: > we should wait dio requests to finish before inode lock in > ocfs2_setattr(), otherwise the following deadlock will be happened: > process 1 process 2 process 3 > truncate file 'A' end_io of writing file 'A' receiving the bast messages > ocfs2_setattr > ocfs2_inode_lock_tracker > ocfs2_inode_lock_full > inode_dio_wait > __inode_dio_wait > -->waiting for all dio > requests finish > dlm_proxy_ast_handler > dlm_do_local_bast > ocfs2_blocking_ast > ocfs2_generic_handle_bast > set OCFS2_LOCK_BLOCKED flag > dio_end_io > dio_bio_end_aio > dio_complete > ocfs2_dio_end_io > ocfs2_dio_end_io_write > ocfs2_inode_lock > __ocfs2_cluster_lock > ocfs2_wait_for_mask > -->waiting for OCFS2_LOCK_BLOCKED > flag to be cleared, that is waiting > for 'process 1' unlocking the inode lock > inode_dio_end > -->here dec the i_dio_count, but will never > be called, so a deadlock happened. > This sounds like something which should be backported into -stable kernels. Do you agree?
Hi Andrew Morton, On 2017/11/2 4:49, Andrew Morton wrote: > On Tue, 31 Oct 2017 14:20:38 +0800 alex chen <alex.chen@huawei.com> wrote: > >> we should wait dio requests to finish before inode lock in >> ocfs2_setattr(), otherwise the following deadlock will be happened: >> process 1 process 2 process 3 >> truncate file 'A' end_io of writing file 'A' receiving the bast messages >> ocfs2_setattr >> ocfs2_inode_lock_tracker >> ocfs2_inode_lock_full >> inode_dio_wait >> __inode_dio_wait >> -->waiting for all dio >> requests finish >> dlm_proxy_ast_handler >> dlm_do_local_bast >> ocfs2_blocking_ast >> ocfs2_generic_handle_bast >> set OCFS2_LOCK_BLOCKED flag >> dio_end_io >> dio_bio_end_aio >> dio_complete >> ocfs2_dio_end_io >> ocfs2_dio_end_io_write >> ocfs2_inode_lock >> __ocfs2_cluster_lock >> ocfs2_wait_for_mask >> -->waiting for OCFS2_LOCK_BLOCKED >> flag to be cleared, that is waiting >> for 'process 1' unlocking the inode lock >> inode_dio_end >> -->here dec the i_dio_count, but will never >> be called, so a deadlock happened. >> > > This sounds like something which should be backported into -stable > kernels. Do you agree? > Yes, I think this problem also exists in the "-stable" tree. Thanks, Alex > > . >
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 6e41fc8..dc455d4 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -1161,6 +1161,13 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) } size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE; if (size_change) { + /* + * Here we should wait dio to finish before inode lock + * to avoid a deadlock between ocfs2_setattr() and + * ocfs2_dio_end_io_write() + */ + inode_dio_wait(inode); + status = ocfs2_rw_lock(inode, 1); if (status < 0) { mlog_errno(status); @@ -1200,8 +1207,6 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) if (status) goto bail_unlock; - inode_dio_wait(inode); - if (i_size_read(inode) >= attr->ia_size) { if (ocfs2_should_order_data(inode)) { status = ocfs2_begin_ordered_truncate(inode,