Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs to avoid deadlock.

[Qu Wenruo <quwenruo@cn.fujitsu.com>]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on 
frozen fs to avoid deadlock.
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@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()

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);