Hi Josef,
this problem could not happen when find_free_extent() was receiving a
transaction handle (which was changed in "Btrfs: avoid starting a
transaction in the write path"), correct? Because it would have used
the passed transaction handle to do the chunk allocation, and thus
would not need to do join_transaction/end_transaction leading to
recursive run_delayed_refs call.
Alex.
On Fri, Mar 7, 2014 at 3:01 AM, Josef Bacik <jbacik@xxxxxx> wrote:
> Zach found this deadlock that would happen like this
>
> btrfs_end_transaction <- reduce trans->use_count to 0
> btrfs_run_delayed_refs
> btrfs_cow_block
> find_free_extent
> btrfs_start_transaction <- increase trans->use_count to 1
> allocate chunk
> btrfs_end_transaction <- decrease trans->use_count to 0
> btrfs_run_delayed_refs
> lock tree block we are cowing above ^^
>
> We need to only decrease trans->use_count if it is above 1, otherwise leave it
> alone. This will make nested trans be the only ones who decrease their added
> ref, and will let us get rid of the trans->use_count++ hack if we have to commit
> the transaction. Thanks,
>
> cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Zach Brown <zab@xxxxxxxxxx>
> Signed-off-by: Josef Bacik <jbacik@xxxxxx>
> ---
> fs/btrfs/transaction.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 34cd831..b05bf58 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -683,7 +683,8 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
> int lock = (trans->type != TRANS_JOIN_NOLOCK);
> int err = 0;
>
> - if (--trans->use_count) {
> + if (trans->use_count > 1) {
> + trans->use_count--;
> trans->block_rsv = trans->orig_rsv;
> return 0;
> }
> @@ -731,17 +732,10 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
> }
>
> if (lock && ACCESS_ONCE(cur_trans->state) == TRANS_STATE_BLOCKED) {
> - if (throttle) {
> - /*
> - * We may race with somebody else here so end up having
> - * to call end_transaction on ourselves again, so inc
> - * our use_count.
> - */
> - trans->use_count++;
> + if (throttle)
> return btrfs_commit_transaction(trans, root);
> - } else {
> + else
> wake_up_process(info->transaction_kthread);
> - }
> }
>
> if (trans->type & __TRANS_FREEZABLE)
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
