Skip to contentSkip to conversation

btrfs: fix deadlock when doing reservation

PATCH In review Miao Xie 3 participants 6 messages Jun 15, 2011 – Jun 16, 2011 Btrfs file system

lore
Share & tools
lore

Miao Xie

author
#0 lore raw

PATCH 2/2 btrfs: fix deadlock when doing reservation

The following deadlock may happen when doing reservation for metadata:

Task0 Flush thread Task1
start_transaction()
shrink_delalloc()
writeback_inodes_sb_nr()
wait for flush thread
end.
btrfs_writepages()
cow_file_range()
btrfs_commit_transaction
wait num_writer == 1 (wait Task0 end
transaction)
start_transaction()
wait trans commit
end

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handle has been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>
---
 fs/btrfs/extent-tree.c |   25 +++++++++++++----------
 fs/btrfs/transaction.c |   51 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 50 insertions(+), 26 deletions(-)
fs/btrfs/extent-tree.c +14 -11
index b42efc2..eefa432 100644
@@ -3290,9 +3290,11 @@ again:
 
 /*
  * shrink metadata reservation for delalloc
+ *
+ * NOTE: This function can not run in the transaction context, or deadlock
+ *       will happen.
  */
-static int shrink_delalloc(struct btrfs_trans_handle *trans,
-			   struct btrfs_root *root, u64 to_reclaim, int sync)
+static int shrink_delalloc(struct btrfs_root *root, u64 to_reclaim)
 {
 	struct btrfs_block_rsv *block_rsv;
 	struct btrfs_space_info *space_info;

@@ -3338,9 +3340,6 @@ static int shrink_delalloc(struct btrfs_trans_handle *trans,
 		if (reserved == 0 || reclaimed >= max_reclaim)
 			break;
 
-		if (trans && trans->transaction->blocked)
-			return -EAGAIN;
-
 		time_left = schedule_timeout_interruptible(1);
 
 		/* We were interrupted, exit */

@@ -3449,12 +3448,16 @@ again:
 	/*
 	 * We do synchronous shrinking since we don't actually unreserve
 	 * metadata until after the IO is completed.
+	 *
+	 * shrink_delalloc() can not run in the transaction context.
 	 */
-	ret = shrink_delalloc(trans, root, num_bytes, 1);
-	if (ret > 0)
-		return 0;
-	else if (ret < 0)
-		goto out;
+	if (!trans || !trans->transaction) {
+		ret = shrink_delalloc(root, num_bytes);
+		if (ret > 0)
+			return 0;
+		else if (ret < 0)
+			goto out;
+	}
 
 	/*
 	 * So if we were overcommitted it's possible that somebody else flushed

@@ -3989,7 +3992,7 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes)
 	block_rsv_add_bytes(block_rsv, to_reserve, 1);
 
 	if (block_rsv->size > 512 * 1024 * 1024)
-		shrink_delalloc(NULL, root, to_reserve, 0);
+		shrink_delalloc(root, to_reserve);
 
 	return 0;
 }
fs/btrfs/transaction.c +36 -15

84 lines not on this page · raw message

Signature · 1 line
1.7.4
#1 lore raw
Quoted 18 lines · Miao Xie

The following deadlock may happen when doing reservation for metadata:

Task0 Flush thread Task1
start_transaction()
shrink_delalloc()
writeback_inodes_sb_nr()
wait for flush thread
end.
btrfs_writepages()
cow_file_range()
btrfs_commit_transaction
wait num_writer == 1 (wait Task0 end
transaction)
start_transaction()
wait trans commit
end

Quoted 6 lines

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handle has been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>

I've already taken care of this in

[PATCH 1/2] Btrfs: do transaction space reservation before joining the transaction

Thanks,

Josef

#2 lore raw

2011/6/15 Josef Bacik <[email protected]>:

Quoted 2 lines · Miao Xie
Quoted 1 line

The following deadlock may happen when doing reservation for metadat=

a:

Quoted 2 lines
Quoted 2 lines

Task0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Flush thread =A0=

=A0 =A0 =A0 =A0 =A0Task1

Quoted 6 lines
Quoted 6 lines

start_transaction()
=A0 shrink_delalloc()
=A0 =A0 writeback_inodes_sb_nr()
=A0 =A0 =A0 wait for flush thread
=A0 =A0 =A0 end.
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 btrfs_wr=

itepages()

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cow_=

file_range()

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 btrfs_commit_transaction

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wait num_writer =3D=3D 1

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (wait Task0 end

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0transaction)

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 star=

t_transaction()

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

wait trans commit

Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

end

Quoted 4 lines
Quoted 4 lines

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handle =

has

Quoted 7 lines
Quoted 3 lines

been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>

I've already taken care of this in

[PATCH 1/2] Btrfs: do transaction space reservation before joining th=

e

Quoted 5 lines

transaction

Thanks,

Josef

I've been trying to run down an issue with btrfs freezing with the 3.0_rc btrfs patch set. I've found a test case that repeatably freezes up on my system, and have been surveying the patches on the list to see if the issue has already been resolved.

I've been successful in addressing the deadlock by applying Miao Xie's patches (I've tested with both "[1/2] btrfs: fix wrong reservation when doing delayed inode operations" and "[2/2] btrfs: fix deadlock when doing reservation").

I've only been partially successful in running my test case with Josef Bacik's patches (the deadlock is cleared; however, now I'm running into a premature ENOSPC).

When evaluating Josef's patches, I've applied:
Btrfs: account for space reservations properly V2
Btrfs: fix btrfs_update_reserved_bytes usage
[1/2] Btrfs: do transaction space reservation before joining the transa= ction
[2/2] Btrfs: serialize flushers in reserve_metadata_bytes

My test kernels are 2.6.39.1 kernels merged with the for-linus branch of Chris' Btrfs unstable kernel tree.

If Miao Xie's patch is not going to be adopted, I'd like to get a clearer idea of the specific patches that are being put forward to address this issue, and I'll apply my test case against those patches.

It's possible that I've applied too many of Josef's proposed patches from the Mailing List, so let me know if I need to go another direction.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 50 lines · Mitch Harder

2011/6/15 Josef Bacik <[email protected]>:

Quoted 26 lines · Miao Xie
Quoted 18 lines

The following deadlock may happen when doing reservation for metadata:

Task0 Flush thread Task1
start_transaction()
shrink_delalloc()
writeback_inodes_sb_nr()
wait for flush thread
end.
btrfs_writepages()
cow_file_range()
btrfs_commit_transaction
wait num_writer == 1 (wait Task0 end
transaction)
start_transaction()
wait trans commit
end

Quoted 6 lines

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handle has been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>
Quoted 8 lines

I've already taken care of this in

[PATCH 1/2] Btrfs: do transaction space reservation before joining the transaction

Thanks,

Josef

I've been trying to run down an issue with btrfs freezing with the 3.0_rc btrfs patch set. I've found a test case that repeatably freezes up on my system, and have been surveying the patches on the list to see if the issue has already been resolved.

I've been successful in addressing the deadlock by applying Miao Xie's patches (I've tested with both "[1/2] btrfs: fix wrong reservation when doing delayed inode operations" and "[2/2] btrfs: fix deadlock when doing reservation").

I've only been partially successful in running my test case with Josef Bacik's patches (the deadlock is cleared; however, now I'm running into a premature ENOSPC).

Quoted 6 lines

When evaluating Josef's patches, I've applied:
Btrfs: account for space reservations properly V2
Btrfs: fix btrfs_update_reserved_bytes usage
[1/2] Btrfs: do transaction space reservation before joining the transaction [2/2] Btrfs: serialize flushers in reserve_metadata_bytes

So drop those first 2, they are wrong and why they are giving you early enospc. The last two are what you want and should fix your deadlock. Thanks,

JOsef

#4 lore raw
Quoted 4 lines · Josef Bacik
Quoted 3 lines · Mitch Harder

2011/6/15 Josef Bacik <[email protected]>:

Quoted 2 lines · Miao Xie
Quoted 1 line

The following deadlock may happen when doing reservation for metad=

ata:

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Task0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Flush thread=

=A0 =A0 =A0 =A0 =A0 =A0Task1

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

start_transaction()
=A0 shrink_delalloc()
=A0 =A0 writeback_inodes_sb_nr()
=A0 =A0 =A0 wait for flush thread
=A0 =A0 =A0 end.
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 btrfs_=

writepages()

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 co=

w_file_range()

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 btrfs_commit_transaction

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wait num_writer =3D=3D 1

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (wait Task0 end

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0transaction)

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st=

art_transaction()

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

wait trans commit

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

end

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handl=

e has

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>

I've already taken care of this in

[PATCH 1/2] Btrfs: do transaction space reservation before joining =

the

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

transaction

Thanks,

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

Josef

I've been trying to run down an issue with btrfs freezing with the 3.0_rc btrfs patch set. =A0I've found a test case that repeatably freezes up on my system, and have been surveying the patches on the list to see if the issue has already been resolved.

I've been successful in addressing the deadlock by applying Miao Xie=

's

Quoted 5 lines
Quoted 5 lines

patches (I've tested with both "[1/2] btrfs: fix wrong reservation when doing delayed inode operations" and "[2/2] btrfs: fix deadlock when doing reservation").

I've only been partially successful in running my test case with Jos=

ef

Quoted 7 lines
Quoted 7 lines

Bacik's patches (the deadlock is cleared; however, now I'm running into a premature ENOSPC).

When evaluating Josef's patches, I've applied:
Btrfs: account for space reservations properly V2
Btrfs: fix btrfs_update_reserved_bytes usage
[1/2] Btrfs: do transaction space reservation before joining the tra=

nsaction

Quoted 4 lines
Quoted 2 lines

[2/2] Btrfs: serialize flushers in reserve_metadata_bytes

So drop those first 2, they are wrong and why they are giving you ear=

ly

Quoted 1 line

enospc. =A0The last two are what you want and should fix your deadloc=

k.

Quoted 2 lines

Thanks,

Confirmed.

The deadlock is cleared in my test case when applying just the "[1/2]
Btrfs: do transaction space reservation before joining the transaction" patch and the "[2/2] Btrfs: serialize flushers in reserve_metadata_bytes" patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
Quoted 64 lines · Mitch Harder
Quoted 58 lines · Josef Bacik
Quoted 50 lines · Mitch Harder

2011/6/15 Josef Bacik <[email protected]>:

Quoted 26 lines · Miao Xie
Quoted 18 lines

The following deadlock may happen when doing reservation for metadata:

Task0 Flush thread Task1
start_transaction()
shrink_delalloc()
writeback_inodes_sb_nr()
wait for flush thread
end.
btrfs_writepages()
cow_file_range()
btrfs_commit_transaction
wait num_writer == 1 (wait Task0 end
transaction)
start_transaction()
wait trans commit
end

Quoted 6 lines

Task0 -> Flush thread -> Task1 -> Task0

Fix the above deadlock by doing reservation before the trans handle has been joined into the transaction.

Signed-off-by Miao Xie <[email protected]>
Quoted 8 lines

I've already taken care of this in

[PATCH 1/2] Btrfs: do transaction space reservation before joining the transaction

Thanks,

Josef

I've been trying to run down an issue with btrfs freezing with the 3.0_rc btrfs patch set. I've found a test case that repeatably freezes up on my system, and have been surveying the patches on the list to see if the issue has already been resolved.

I've been successful in addressing the deadlock by applying Miao Xie's patches (I've tested with both "[1/2] btrfs: fix wrong reservation when doing delayed inode operations" and "[2/2] btrfs: fix deadlock when doing reservation").

I've only been partially successful in running my test case with Josef Bacik's patches (the deadlock is cleared; however, now I'm running into a premature ENOSPC).

Quoted 6 lines

When evaluating Josef's patches, I've applied:
Btrfs: account for space reservations properly V2
Btrfs: fix btrfs_update_reserved_bytes usage
[1/2] Btrfs: do transaction space reservation before joining the transaction [2/2] Btrfs: serialize flushers in reserve_metadata_bytes

Quoted 4 lines

So drop those first 2, they are wrong and why they are giving you early enospc. The last two are what you want and should fix your deadlock. Thanks,

Quoted 6 lines

Confirmed.

The deadlock is cleared in my test case when applying just the "[1/2]
Btrfs: do transaction space reservation before joining the transaction" patch and the "[2/2] Btrfs: serialize flushers in reserve_metadata_bytes" patch.

Perfect, thanks,

Josef