* [Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr()
@ 2017-10-31 6:20 alex chen
2017-11-01 20:49 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: alex chen @ 2017-10-31 6:20 UTC (permalink / raw)
To: ocfs2-devel
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 <alex.chen@huawei.com>
Reviewed-by: Jun Piao <piaojun@huawei.com>
Reviewed-by: Joseph Qi <jiangqi903@gmail.com>
Acked-by: Changwei Ge <ge.changwei@h3c.com>
---
fs/ocfs2/file.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
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,
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr()
2017-10-31 6:20 [Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr() alex chen
@ 2017-11-01 20:49 ` Andrew Morton
2017-11-02 3:03 ` alex chen
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2017-11-01 20:49 UTC (permalink / raw)
To: ocfs2-devel
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?
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr()
2017-11-01 20:49 ` Andrew Morton
@ 2017-11-02 3:03 ` alex chen
0 siblings, 0 replies; 3+ messages in thread
From: alex chen @ 2017-11-02 3:03 UTC (permalink / raw)
To: ocfs2-devel
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
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-02 3:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-31 6:20 [Ocfs2-devel] [PATCH v2] ocfs2: should wait dio before inode lock in ocfs2_setattr() alex chen
2017-11-01 20:49 ` Andrew Morton
2017-11-02 3:03 ` alex chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.