[Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr()

alex chen

author

2017-10-31 06:20 UTC #0 lore raw

PATCH v2 1/1 ocfs2: should wait dio before inode lock in ocfs2_setattr()

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.

Signed-off-by Alex Chen <[email protected]>

Reviewed-by Jun Piao <[email protected]>

Reviewed-by Joseph Qi <[email protected]>

Acked-by Changwei Ge <[email protected]>

---
 fs/ocfs2/file.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

fs/ocfs2/file.c +7 -2

index 6e41fc8..dc455d4 100644
@@ -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,

Signature · 1 line

1.9.5.msysgit.1

Andrew Morton

2017-11-01 20:49 UTC #1 lore raw

PATCH v2 1/1 ocfs2: should wait dio before inode lock in ocfs2_setattr()

Quoted 22 lines · On Tue, 31 Oct 2017 14:20:38 +0800 alex chen <[email protected]> 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

Quoted 9 lines

__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?

alex chen

author

2017-11-02 03:03 UTC #2 lore raw

PATCH v2 1/1 ocfs2: should wait dio before inode lock in ocfs2_setattr()

Hi Andrew Morton,

Quoted 34 lines · Andrew Morton

Quoted 22 lines · On Tue, 31 Oct 2017 14:20:38 +0800 alex chen <[email protected]> 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
Quoted 9 lines
__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.

Quoted 3 lines

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

Quoted 3 lines

.