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. > > For example, if we change the features/label through sysfs, and then umount the fs, It is different from pending change. 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
