Thanks for commenting Josef. I hope your head will get better:) Actually, while re-looking at the code, I see that there are couple of "goto cleanup;", before we ensure that all the writers have detached from the committing transaction. So Liu's code is still needed, looks like. Thanks, Alex. On Mon, Jun 24, 2013 at 7:24 PM, Josef Bacik <jbacik@xxxxxxxxxxxx> wrote: > On Sun, Jun 23, 2013 at 09:52:14PM +0300, Alex Lyakas wrote: >> Hello Josef, Liu, >> I am reviewing commits in the mainline tree: >> >> e4a2bcaca9643e7430207810653222fc5187f2be Btrfs: if we aren't >> committing just end the transaction if we error out >> (call end_transaction() instead of goto cleanup_transaction) - Josef >> >> f094ac32aba3a51c00e970a2ea029339af2ca048 Btrfs: fix NULL pointer after >> aborting a transaction >> (wait until all writers detach, before setting running_transaction to >> NULL) - Liu >> >> 66b6135b7cf741f6f44ba938b27583ea3b83bd12 Btrfs: avoid deadlock on >> transaction waiting list >> (check if transaction was already removed from the transactions list) - Liu >> >> Josef, according to your fix, if the committer encounters a problem >> early, it just doesn't commit. Instead it aborts the transaction >> (possibly setting FS to read-only) and detaches from the transaction. >> So if this was the only committer (e.g., the transaction_kthread), >> then transaction commit will not happen at all. Is this what you >> intended? So then the user will notice that FS went read-only, and she >> will unmount the FS, and transaction will be cleaned up in >> btrfs_error_commit_super()=>btrfs_cleanup_transaction(), and not in >> cleanup_transaction() via btrfs_commit_transaction(). Is my >> understanding correct? >> >> Liu, it looks like after having Josef's fix, the above two commits of >> yours are not strictly needed, right? Because Josef's fix ensures that >> only the "real" committer will call cleanup_transaction(), so at this >> point there is only one writer attached to the transaction, which is >> the committer itself (your fixes doesn't hurt though). Is that >> correct? >> > > I've looked at the patches and I'm going to say yes with the caveat that I > stopped thinking about it when my head started hurting :). Thanks, > > Josef -- 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
