diff mbox

Btrfs: fix deadlock with freeze and sync V2

Message ID 1345834383-2172-1-git-send-email-jbacik@fusionio.com (mailing list archive)
State New, archived
Headers show

Commit Message

Josef Bacik Aug. 24, 2012, 6:53 p.m. UTC
We can deadlock with freeze right now because we unconditionally start a
transaction in our ->sync_fs() call.  To fix this just check and see if we
have a running transaction to commit.  This saves us from the deadlock
because at this point we'll have the umount sem for the sb so we're safe
from freezes coming in after we've done our check.  With this patch the
freeze xfstests no longer deadlocks.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
V1->V2: got a little overzealous and cleaned up the ret thing without actually
removing the now unused ret variable.

 fs/btrfs/super.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

Comments

Liu Bo Aug. 25, 2012, 2:32 a.m. UTC | #1
On 08/25/2012 02:53 AM, Josef Bacik wrote:
> We can deadlock with freeze right now because we unconditionally start a
> transaction in our ->sync_fs() call.  To fix this just check and see if we
> have a running transaction to commit.  This saves us from the deadlock
> because at this point we'll have the umount sem for the sb so we're safe
> from freezes coming in after we've done our check.  With this patch the
> freeze xfstests no longer deadlocks.  Thanks,
> 

Actually I have a same patch pending to send...

Anyway, I'm happy with the bug has been fixed.

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
> V1->V2: got a little overzealous and cleaned up the ret thing without actually
> removing the now unused ret variable.
> 
>  fs/btrfs/super.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 7b65fe8..83d6f9f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -838,7 +838,6 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
>  	struct btrfs_trans_handle *trans;
>  	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
>  	struct btrfs_root *root = fs_info->tree_root;
> -	int ret;
>  
>  	trace_btrfs_sync_fs(wait);
>  
> @@ -849,11 +848,17 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
>  
>  	btrfs_wait_ordered_extents(root, 0, 0);
>  
> -	trans = btrfs_start_transaction(root, 0);
> +	spin_lock(&fs_info->trans_lock);
> +	if (!fs_info->running_transaction) {
> +		spin_unlock(&fs_info->trans_lock);
> +		return 0;
> +	}
> +	spin_unlock(&fs_info->trans_lock);
> +
> +	trans = btrfs_join_transaction(root);
>  	if (IS_ERR(trans))
>  		return PTR_ERR(trans);
> -	ret = btrfs_commit_transaction(trans, root);
> -	return ret;
> +	return btrfs_commit_transaction(trans, root);
>  }
>  
>  static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 7b65fe8..83d6f9f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -838,7 +838,6 @@  int btrfs_sync_fs(struct super_block *sb, int wait)
 	struct btrfs_trans_handle *trans;
 	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
 	struct btrfs_root *root = fs_info->tree_root;
-	int ret;
 
 	trace_btrfs_sync_fs(wait);
 
@@ -849,11 +848,17 @@  int btrfs_sync_fs(struct super_block *sb, int wait)
 
 	btrfs_wait_ordered_extents(root, 0, 0);
 
-	trans = btrfs_start_transaction(root, 0);
+	spin_lock(&fs_info->trans_lock);
+	if (!fs_info->running_transaction) {
+		spin_unlock(&fs_info->trans_lock);
+		return 0;
+	}
+	spin_unlock(&fs_info->trans_lock);
+
+	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
-	ret = btrfs_commit_transaction(trans, root);
-	return ret;
+	return btrfs_commit_transaction(trans, root);
 }
 
 static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)