Re: [PATCH] Btrfs: place ordered operations on a per transaction list

[Josef Bacik <jbacik@fusionio.com>]

On Mon, Feb 18, 2013 at 04:22:09AM -0700, Miao Xie wrote:
> On wed, 13 Feb 2013 11:13:22 -0500, Josef Bacik wrote:
> > Miao made the ordered operations stuff run async, which introduced a
> > deadlock where we could get somebody (sync) racing in and committing the
> > transaction while a commit was already happening.  The new committer would
> > try and flush ordered operations which would hang waiting for the commit to
> > finish because it is done asynchronously and no longer inherits the callers
> > trans handle.  To fix this we need to make the ordered operations list a per
> > transaction list.  We can get new inodes added to the ordered operation list
> > by truncating them and then having another process writing to them, so this
> > makes it so that anybody trying to add an ordered operation _must_ start a
> > transaction in order to add itself to the list, which will keep new inodes
> > from getting added to the ordered operations list after we start committing.
> > This should fix the deadlock and also keeps us from doing a lot more work
> > than we need to during commit.  Thanks,
> 
> Firstly, thanks to deal with the bug which was introduced by my patch.
> 
> But comparing with this fix method, I prefer the following one because:
> - we won't worry the similar problem if we add more work during commit
>   in the future.
> - it is unnecessary to get a new handle and commit it if the transaction
>   is under the commit.

Mine has the benefit of not making a committing transaction flush more stuff
that it doesn't need to, so I think I'll keep mine as well, but I agree yours is
good for the attach case as well.  So can you send this along properly with a
signed off and such and we can have our cake and eat it too.  Thanks,

Josef