On Fri, 30 Jan 2015 10:51:52 +0800, Qu Wenruo wrote: > > -------- Original Message -------- > Subject: Re: [PATCH RESEND v4 2/8] btrfs: Make btrfs_parse_options() parse > mount > option in a atomic way > From: Miao Xie <miao...@huawei.com> > To: Qu Wenruo <quwen...@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org> > Date: 2015年01月30日 10:06 >> On Fri, 30 Jan 2015 09:33:17 +0800, Qu Wenruo wrote: >>> -------- Original Message -------- >>> Subject: Re: [PATCH RESEND v4 2/8] btrfs: Make btrfs_parse_options() parse >>> mount >>> option in a atomic way >>> From: Miao Xie <miao...@huawei.com> >>> To: Qu Wenruo <quwen...@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org> >>> Date: 2015年01月30日 09:29 >>>> On Fri, 30 Jan 2015 09:20:46 +0800, Qu Wenruo wrote: >>>>>> Here need ACCESS_ONCE to wrap info->mount_opt, or the complier might use >>>>>> info->mount_opt instead of new_opt. >>>>> Thanks for pointing out this one. >>>>>> But I worried that this is not key reason of the wrong space cache. Could >>>>>> you explain the race condition which caused the wrong space cache? >>>>>> >>>>>> Thanks >>>>>> Miao >>>>> CPU0: >>>>> remount() >>>>> |- sync_fs() <- after sync_fs() we can start new trans >>>>> |- btrfs_parse_options() CPU1: >>>>> |- start_transaction() >>>>> |- Do some bg allocation, not recorded in >>>>> space_cache. >>>> I think it is a bug if a free space is not recorded in space cache. Could >>>> you >>>> explain why it is not recorded? >>>> >>>> Thanks >>>> Miao >>> IIRC, in that window, the fs_info->mount_opt's SPACE_CACHE bit is cleared. >>> So space cache is not recorded. >> SPACE_CACHE is used to control cache write out, not in-memory cache. All the >> free space should be recorded in in-memory cache.And when we write out >> the in-memory space cache, we need protect the space cache from changing. >> >> Thanks >> Miao > You're right, the wrong space cache problem is not caused by the non-atomic > mount option problem. > But the atomic mount option change with per-transaction mount option will at > least make it disappear > when using nospace_cache mount option.
But we need fix a problem, not hide a problem. Thanks Miao > > Thanks, > Qu >> >>> Thanks, >>> Qu >>>>> |- set SPACE_CACHE bit due to cache_gen >>>>> >>>>> |- commit_transaction() >>>>> |- write space cache and update cache_gen. >>>>> but since some of it is not recorded in >>>>> space >>>>> cache, >>>>> the space cache missing some records. >>>>> |- clear SPACE_CACHE bit dut to nospace_cache >>>>> >>>>> So the space cache is wrong. >>>>> >>>>> Thanks, >>>>> Qu >>>>>>> + } >>>>>>> kfree(orig); >>>>>>> return ret; >>>>>>> } >>>>>>> >>>>> . >>>>> >>> > > . > -- 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
