-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on
frozen fs to avoid deadlock.
From: Miao Xie <miao...@huawei.com>
To: Qu Wenruo <quwen...@cn.fujitsu.com>, <dste...@suse.cz>,
<linux-btrfs@vger.kernel.org>
Date: 2015年01月20日 11:06
On Tue, 20 Jan 2015 10:53:05 +0800, Qu Wenruo wrote:
Add CC to Miao Xie <miao...@huawei.com>
-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs to
avoid deadlock.
From: Qu Wenruo <quwen...@cn.fujitsu.com>
To: dste...@suse.cz, linux-btrfs@vger.kernel.org, Miao Xie
<mi...@cn.fujitsu.com>
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 <dste...@suse.cz>
To: Qu Wenruo <quwen...@cn.fujitsu.com>
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()
I don't like this fix.
I think we should deal with those pending changes when we freeze a filesystem.
or we break the rule of fs freeze.
I am afraid handling it in btrfs_freeze() won't help.
Case like freeze() -> change_feature -> sync() -> unfreeze() will still
deadlock in sync().
Even cleared the pending changes in freeze(), it can still be set
through sysfs interface even the fs is frozen.
And in fact, if we put the things like attach/create a transaction into
a workqueue, we will not break
the freeze rule.
Since if the fs is frozen, there is no running transaction and we need
to create a new one,
that will call sb_start_intwrite(), which will sleep until the fs is
unfreeze.
Thanks,
Qu
Thanks
Miao
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
