Re: [PATCH] Btrfs: fix freeing delayed ref head while still holding its mutex

[Josef Bacik <jbacik@fusionio.com>]

On Wed, Jan 30, 2013 at 03:16:35PM -0700, Zach Brown wrote:
> On Wed, Jan 30, 2013 at 04:06:18PM -0500, Josef Bacik wrote:
> > I hit this error when reproducing a bug that would end in a transaction
> > abort.  We take the delayed ref head's mutex to keep anybody from processing
> > it while we're destroying it, but we fail to drop the mutex before we carry
> > on and free the damned thing.  Fix this by doing the remove logic for the
> > head ourselves and unlock the mutex, that way we can avoid use after free's
> > or hung tasks waiting on that mutex to come back so they know the delayed
> > ref completed.  Thanks,
> > 
> 
> > +			ref->in_tree = 0;
> > +			rb_erase(&ref->rb_node, &delayed_refs->root);
> > +			delayed_refs->num_entries--;
> > +			mutex_unlock(&head->mutex);
> > +		} else {
> > +			ref->in_tree = 0;
> > +			rb_erase(&ref->rb_node, &delayed_refs->root);
> > +			delayed_refs->num_entries--;
> 
> Do you really need to duplicate the removal under the mutex?  Isn't all
> that protected by the delayed_refs->lock?
> 
> Isn't it enough to just add the mutex_unlock()?
> 

Yeah I could re-arrange I guess, either way it's not pretty but I guess not
duplicating stuff is better.  Thanks,

Josef