-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on
frozen fs to avoid deadlock.
From: Miao Xie <miaoxie@xxxxxxxxxx>
To: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>, Chris Mason <clm@xxxxxx>
Date: 2015年01月21日 11:26
On Wed, 21 Jan 2015 11:15:41 +0800, Qu Wenruo wrote:
-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs to
avoid deadlock.
From: Miao Xie <miaoxie@xxxxxxxxxx>
To: Chris Mason <clm@xxxxxx>, Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
Date: 2015年01月21日 11:10
On Tue, 20 Jan 2015 20:10:56 -0500, Chris Mason wrote:
On Tue, Jan 20, 2015 at 8:09 PM, Qu Wenruo <quwenruo@xxxxxxxxxxxxxx> wrote:
-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs
to avoid deadlock.
From: Chris Mason <clm@xxxxxx>
To: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
Date: 2015年01月21日 09:05
On Tue, Jan 20, 2015 at 7:58 PM, Qu Wenruo <quwenruo@xxxxxxxxxxxxxx> wrote:
-------- 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月21日 01:13
On Mon, Jan 19, 2015 at 03:42:41PM +0800, Qu Wenruo wrote:
--- 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;
But what if someone freezes the FS after __sb_end_write() and before
btrfs_start_transaction()? I don't see what keeps new freezers from
coming in.
-chris
Either VFS::freeze_super() and VFS::syncfs() will hold the s_umount mutex, so
freeze will not happen
during sync.
You're right. I was worried about the sync ioctl, but the mutex won't be held
there to deadlock against. We'll be fine.
There is another problem which is introduced by pending change. That is we will
start and commmit a transaction by changing pending mount option after we set
the fs to be R/O.
Oh, I missed this problem.
I think it is better that we don't start a new transaction for pending changes
which are set after the transaction is committed, just make them be handled by
the next transaction,
This will cause another problem, nobody can ensure there will be next
transaction and the change may
never to written into disk.
First, the pending changes is mount option, that is in-memory data.
Second, the same problem would happen after you freeze fs.
Pending changes are *not* only mount options. Feature change and label
change are also pending changes if using sysfs.
Normal ioctl label changing is not affected.
For freeze, it's not the same problem since the fs will be unfreeze
sooner or later and transaction will be initiated.
For example, if we change the features/label through sysfs, and then umount the fs,
It is different from pending change.
No, now features/label changing using sysfs both use pending changes to
do the commit.
See BTRFS_PENDING_COMMIT bit.
So freeze -> change features/label -> sync will still cause the deadlock
in the same way,
and you can try it yourself.
Thanks,
Qu
If you want to change features/label, you should get write permission and make
sure the fs is not be freezed because those are on-disk data. So the problem
doesn't exist, or there is a bug.
Thanks
Miao
since there is no write, there is no running transaction and if we don't start a
new transaction,
it won't be flushed to disk.
Thanks,
Qu
the reason is:
- Make the behavior of the fs be consistent(both freezed fs and unfreezed fs)
- Data on the disk is right and integrated
Thanks
Miao
.
--
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
