Btrfs devel - Nov 2012 - [PATCH 1/2] Btrfs: fix a deadlock in aborting transaction due to ENOSPC

When committing a transaction, we may bail out of running delayed refs
due to ENOSPC, and then abort the current transaction to flip into readonly.

But we''ll hit a deadlock on ref head''s lock since we forget to
release
its lock and other cleanup stuff.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/extent-tree.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..e0c4809 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2314,6 +2314,9 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
 				kfree(extent_op);
 
 				if (ret) {
+					list_del_init(&locked_ref->cluster);
+					mutex_unlock(&locked_ref->mutex);
+
 					printk(KERN_DEBUG "btrfs: run_delayed_extent_op returned %d\n",
ret);
 					spin_lock(&delayed_refs->lock);
 					return ret;
@@ -2356,6 +2359,10 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
 		count++;
 
 		if (ret) {
+			if (locked_ref) {
+				list_del_init(&locked_ref->cluster);
+				mutex_unlock(&locked_ref->mutex);
+			}
 			printk(KERN_DEBUG "btrfs: run_one_delayed_ref returned %d\n",
ret);
 			spin_lock(&delayed_refs->lock);
 			return ret;
-- 
1.7.7.6

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

When creating a snapshot, failing to commit a transaction can end up
with aborting the transaction, following by doing a cleanup for it, where
we''ll free all snapshots pending to disk.

So we check it and avoid double free on pending snapshots.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/ioctl.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 8fcf9a5..4e1a1ce 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -571,8 +571,12 @@ static int create_snapshot(struct btrfs_root *root, struct
dentry *dentry,
 		ret = btrfs_commit_transaction(trans,
 					       root->fs_info->extent_root);
 	}
-	if (ret)
+	if (ret) {
+		/* cleanup_transaction has freed this for us */
+		if (trans->aborted)
+			pending_snapshot = NULL;
 		goto fail;
+	}
 
 	ret = pending_snapshot->error;
 	if (ret)
-- 
1.7.7.6

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

On Mon, Nov 05, 2012 at 08:42:08PM +0800, Liu Bo wrote:
> When committing a transaction, we may bail out of running delayed refs
> due to ENOSPC, and then abort the current transaction to flip into
readonly.
> 
> But we''ll hit a deadlock on ref head''s lock since we
forget to release
> its lock and other cleanup stuff.
Miao sent a patch for this on Tue, 08 May 2012

http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg16361.html

and it does more things than just the unlock, I can''t tell which one of
the patches is the right one, only letting you know.

david
--
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

On Mon, Nov 05, 2012 at 08:42:09PM +0800, Liu Bo wrote:
> When creating a snapshot, failing to commit a transaction can end up
> with aborting the transaction, following by doing a cleanup for it, where
> we''ll free all snapshots pending to disk.
> 
> So we check it and avoid double free on pending snapshots.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.cz>

I''ve reproduced this as well, so consider it tested.

david
--
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

On Fri, Nov 16, 2012 at 03:19:49PM +0100, David Sterba
wrote:
> On Mon, Nov 05, 2012 at 08:42:08PM +0800, Liu Bo wrote:
> > When committing a transaction, we may bail out of running delayed refs
> > due to ENOSPC, and then abort the current transaction to flip into
readonly.
> > 
> > But we''ll hit a deadlock on ref head''s lock since we
forget to release
> > its lock and other cleanup stuff.
> 
> Miao sent a patch for this on Tue, 08 May 2012
> 
> http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg16361.html
> 
> and it does more things than just the unlock, I can''t tell which
one of
> the patches is the right one, only letting you know.
> 
Thanks for pointing that, David.  Yes, that patch will work.

Miao, can you send it here again?
[PATCH] Btrfs: fix deadlock when the process of delayed refs fails

thanks,
liubo

> david
> --
> 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
--
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

On 	mon, 19 Nov 2012 15:58:55 +0800, Liu Bo wrote:
> On Fri, Nov 16, 2012 at 03:19:49PM +0100, David Sterba wrote:
>> On Mon, Nov 05, 2012 at 08:42:08PM +0800, Liu Bo wrote:
>>> When committing a transaction, we may bail out of running delayed
refs
>>> due to ENOSPC, and then abort the current transaction to flip into
readonly.
>>>
>>> But we''ll hit a deadlock on ref head''s lock since
we forget to release
>>> its lock and other cleanup stuff.
>>
>> Miao sent a patch for this on Tue, 08 May 2012
>>
>> http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg16361.html
>>
>> and it does more things than just the unlock, I can''t tell
which one of
>> the patches is the right one, only letting you know.
>>
> 
> Thanks for pointing that, David.  Yes, that patch will work.
> 
> Miao, can you send it here again?
> [PATCH] Btrfs: fix deadlock when the process of delayed refs fails
OK, I will send it out again.

Thanks
Miao
> 
> thanks,
> liubo
> 
> 
>> david
>> --
>> 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
> --
> 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
> 
--
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