On Tue, 20 Jan 2015 20:10:56 -0500, Chris Mason wrote: > On Tue, Jan 20, 2015 at 8:09 PM, Qu Wenruo <quwen...@cn.fujitsu.com> wrote: >> >> -------- Original Message -------- >> Subject: Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs >> to avoid deadlock. >> From: Chris Mason <c...@fb.com> >> To: Qu Wenruo <quwen...@cn.fujitsu.com> >> Date: 2015年01月21日 09:05 >>> >>> >>> On Tue, Jan 20, 2015 at 7:58 PM, Qu Wenruo <quwen...@cn.fujitsu.com> wrote: >>>> >>>> -------- 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月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. 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, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
