linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: fdmanana@kernel.org
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 2/3] Btrfs: unpin logs if rename exchange operation fails
Date: Thu,  5 May 2016 02:34:15 +0100	[thread overview]
Message-ID: <1462412056-27878-2-git-send-email-fdmanana@kernel.org> (raw)
In-Reply-To: <1462412056-27878-1-git-send-email-fdmanana@kernel.org>

From: Filipe Manana <fdmanana@suse.com>

If rename exchange operations fail at some point after we pinned any of
the logs, we end up aborting the current transaction but never unpin the
logs, which leaves concurrent tasks that are trying to sync the logs (as
part of an fsync request from user space) blocked forever and preventing
the filesystem from being unmountable.

Fix this by safely unpinning the log.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ab64721..503d749 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9412,6 +9412,8 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 	u64 new_idx = 0;
 	u64 root_objectid;
 	int ret;
+	bool root_log_pinned = false;
+	bool dest_log_pinned = false;
 
 	/* we only allow rename subvolume link between subvolumes */
 	if (old_ino != BTRFS_FIRST_FREE_OBJECTID && root != dest)
@@ -9464,6 +9466,7 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 		if (ret)
 			goto out_fail;
 		btrfs_pin_log_trans(root);
+		root_log_pinned = true;
 	}
 
 	/* And now for the dest. */
@@ -9479,6 +9482,7 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 		if (ret)
 			goto out_fail;
 		btrfs_pin_log_trans(dest);
+		dest_log_pinned = true;
 	}
 
 	/* Update inode version and ctime/mtime. */
@@ -9557,17 +9561,47 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 	if (new_inode->i_nlink == 1)
 		BTRFS_I(new_inode)->dir_index = new_idx;
 
-	if (old_ino != BTRFS_FIRST_FREE_OBJECTID) {
+	if (root_log_pinned) {
 		parent = new_dentry->d_parent;
 		btrfs_log_new_name(trans, old_inode, old_dir, parent);
 		btrfs_end_log_trans(root);
+		root_log_pinned = false;
 	}
-	if (new_ino != BTRFS_FIRST_FREE_OBJECTID) {
+	if (dest_log_pinned) {
 		parent = old_dentry->d_parent;
 		btrfs_log_new_name(trans, new_inode, new_dir, parent);
 		btrfs_end_log_trans(dest);
+		dest_log_pinned = false;
 	}
 out_fail:
+	/*
+	 * If we have pinned a log and an error happened, we unpin tasks
+	 * trying to sync the log and force them to fallback to a transaction
+	 * commit if the log currently contains any of the inodes involved in
+	 * this rename operation (to ensure we do not persist a log with an
+	 * inconsistent state for any of these inodes or leading to any
+	 * inconsistencies when replayed). If the transaction was aborted, the
+	 * abortion reason is propagated to userspace when attempting to commit
+	 * the transaction. If the log does not contain any of these inodes, we
+	 * allow the tasks to sync it.
+	 */
+	if (ret && (root_log_pinned || dest_log_pinned)) {
+		if (btrfs_inode_in_log(old_dir, root->fs_info->generation) ||
+		    btrfs_inode_in_log(new_dir, root->fs_info->generation) ||
+		    btrfs_inode_in_log(old_inode, root->fs_info->generation) ||
+		    (new_inode &&
+		     btrfs_inode_in_log(new_inode, root->fs_info->generation)))
+		    btrfs_set_log_full_commit(root->fs_info, trans);
+
+		if (root_log_pinned) {
+			btrfs_end_log_trans(root);
+			root_log_pinned = false;
+		}
+		if (dest_log_pinned) {
+			btrfs_end_log_trans(dest);
+			dest_log_pinned = false;
+		}
+	}
 	ret = btrfs_end_transaction(trans, root);
 out_notrans:
 	if (new_ino == BTRFS_FIRST_FREE_OBJECTID)
-- 
2.7.0.rc3


  reply	other threads:[~2016-05-05 16:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-05  1:34 [PATCH 1/3] Btrfs: fix inode leak on failure to setup whiteout inode in rename fdmanana
2016-05-05  1:34 ` fdmanana [this message]
2016-05-05  1:34 ` [PATCH 3/3] Btrfs: pin logs earlier when doing a rename exchange operation fdmanana

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=1462412056-27878-2-git-send-email-fdmanana@kernel.org \
    --to=fdmanana@kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    /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).