linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
To: miklos@szeredi.hu
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	xieyongji@bytedance.com,
	Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Subject: [PATCH] fuse: fix deadlock between O_TRUNC open() and non-O_TRUNC open()
Date: Thu, 16 Dec 2021 22:45:58 +0800	[thread overview]
Message-ID: <20211216144558.63931-1-zhangjiachen.jaycee@bytedance.com> (raw)

fuse_finish_open() will be called with FUSE_NOWRITE set in case of atomic
O_TRUNC open(), so commit 76224355db75 ("fuse: truncate pagecache on
atomic_o_trunc") replaced invalidate_inode_pages2() by truncate_pagecache()
in such a case to avoid the A-A deadlock. However, we found another A-B-B-A
deadlock related to the case above, which will cause the xfstests
generic/464 testcase hung in our virtio-fs test environment.

Consider two processes concurrently open one same file, one with O_TRUNC
and another without O_TRUNC. The deadlock case is described below, if
open(O_TRUNC) is already set_nowrite(acquired A), and is trying to lock
a page (acquiring B), open() could have held the page lock (acquired B),
and waiting on the page writeback (acquiring A). This would lead to
deadlocks.

This commit tries to fix it by locking inode in fuse_open_common() even
if O_TRUNC is not set. This introduces a lock C to protect the area with
A-B-B-A the deadlock rick.

open(O_TRUNC)
----------------------------------------------------------------
fuse_open_common
  lock_inode		[C acquire]
  fuse_set_nowrite	[A acquire]

  fuse_finish_open
    truncate_pagecache
      lock_page		[B acquire]
      truncate_inode_page
      unlock_page	[B release]

  fuse_release_nowrite	[A release]
  unlock_inode		[C release]
----------------------------------------------------------------

open()
----------------------------------------------------------------
fuse_open_common
  (lock_inode)		[C acquire, introduced by this commit]

  fuse_finish_open
    invalidate_inode_pages2
      lock_page		[B acquire]
        fuse_launder_page
          fuse_wait_on_page_writeback [A acquire]
      unlock_page	[B release]

  (unlock_inode)	[C release, introduced by this commit]
----------------------------------------------------------------

Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
---
 fs/fuse/file.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 829094451774..73fec30e0a37 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -229,8 +229,10 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	bool is_wb_truncate = (file->f_flags & O_TRUNC) &&
 			  fc->atomic_o_trunc &&
 			  fc->writeback_cache;
-	bool dax_truncate = (file->f_flags & O_TRUNC) &&
+	bool is_dax_truncate = (file->f_flags & O_TRUNC) &&
 			  fc->atomic_o_trunc && FUSE_IS_DAX(inode);
+	bool may_truncate = fc->atomic_o_trunc &&
+			  (fc->writeback_cache || FUSE_IS_DAX(inode));
 
 	if (fuse_is_bad(inode))
 		return -EIO;
@@ -239,12 +241,12 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	if (err)
 		return err;
 
-	if (is_wb_truncate || dax_truncate) {
+	if (may_truncate)
 		inode_lock(inode);
+	if (is_wb_truncate || is_dax_truncate)
 		fuse_set_nowrite(inode);
-	}
 
-	if (dax_truncate) {
+	if (is_dax_truncate) {
 		filemap_invalidate_lock(inode->i_mapping);
 		err = fuse_dax_break_layouts(inode, 0, 0);
 		if (err)
@@ -256,13 +258,13 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 		fuse_finish_open(inode, file);
 
 out:
-	if (dax_truncate)
+	if (is_dax_truncate)
 		filemap_invalidate_unlock(inode->i_mapping);
 
-	if (is_wb_truncate | dax_truncate) {
+	if (is_wb_truncate | is_dax_truncate)
 		fuse_release_nowrite(inode);
+	if (may_truncate)
 		inode_unlock(inode);
-	}
 
 	return err;
 }
-- 
2.20.1


             reply	other threads:[~2021-12-16 14:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-16 14:45 Jiachen Zhang [this message]
2021-12-20 12:49 ` [PATCH] fuse: fix deadlock between O_TRUNC open() and non-O_TRUNC open() Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211216144558.63931-1-zhangjiachen.jaycee@bytedance.com \
    --to=zhangjiachen.jaycee@bytedance.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=xieyongji@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).