Add CC to Miao Xie <miaoxie@xxxxxxxxxx>
-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on
frozen fs to avoid deadlock.
From: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
To: dsterba@xxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, Miao Xie
<miaox@xxxxxxxxxxxxxx>
Date: 2015年01月20日 10:51
-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on
frozen fs to avoid deadlock.
From: David Sterba <dsterba@xxxxxxx>
To: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
Date: 2015年01月19日 22:06
On Mon, Jan 19, 2015 at 03:42:41PM +0800, Qu Wenruo wrote:
The fix is to check if the fs is frozen, if the fs is frozen, just
return and waiting for the next transaction.
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1000,6 +1000,14 @@ int btrfs_sync_fs(struct super_block *sb, int
wait)
*/
if (fs_info->pending_changes == 0)
return 0;
+ /*
+ * Test if the fs is frozen, or start_trasaction
+ * will deadlock on itself.
+ */
+ if (__sb_start_write(sb, SB_FREEZE_FS, false))
+ __sb_end_write(sb, SB_FREEZE_FS);
+ else
+ return 0;
I'm not sure this is the right fix. We should use either
mnt_want_write_file or sb_start_write around the start/commit functions.
The fs may be frozen already, but we also have to catch transition to
that state, or RO remount.
But the deadlock between s_umount and frozen level is a larger problem...
Even Miao mentioned that we can start a transaction in btrfs_freeze(),
but there is still possibility that
we try to change the feature of the frozen btrfs and do sync, again
the deadlock will happen.
Although handling in btrfs_freeze() is also needed, but can't resolve
all the problem.
IMHO the fix is still needed, or at least as a workaround until we
find a real root solution for it
(If nobody want to revert the patchset)
BTW, what about put the pending changes to a workqueue? If we don't
start transaction under
s_umount context like sync_fs()
Thanks,
Qu
Also, returning 0 is not right, the ioctl actually skipped the expected
work.
trans = btrfs_start_transaction(root, 0);
} else {
return PTR_ERR(trans);
--
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
