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 <[email protected]> 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: [email protected] > Reported-by: Zach Brown <[email protected]> > Signed-off-by: Josef Bacik <[email protected]> > --- > 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 [email protected] > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
