Re: [PATCH RESEND v4 2/8] btrfs: Make btrfs_parse_options() parse mount option in a atomic way

[Miao Xie <miaoxie@huawei.com>]

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 <miaoxie@huawei.com>
> To: Qu Wenruo <quwenruo@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

> 
> 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;
>>>>>    }
>>>>>
>>> .
>>>
> 
>