Re: [PATCH v3.1 0/7] Chunk level degradable check

["Austin S. Hemmelgarn" <ahferroin7@gmail.com>]

On 2017-06-26 22:49, Qu Wenruo wrote:
> 
> 
> At 06/27/2017 09:59 AM, Anand Jain wrote:
>>
>>
>> On 06/27/2017 09:05 AM, Qu Wenruo wrote:
>>>
>>>
>>> At 06/27/2017 02:59 AM, David Sterba wrote:
>>>> On Thu, Mar 09, 2017 at 09:34:35AM +0800, Qu Wenruo wrote:
>>>>> Btrfs currently uses num_tolerated_disk_barrier_failures to do global
>>>>> check for tolerated missing device.
>>>>>
>>>>> Although the one-size-fit-all solution is quite safe, it's too strict
>>>>> if data and metadata has different duplication level.
>>>>>
>>>>> For example, if one use Single data and RAID1 metadata for 2 disks, it
>>>>> means any missing device will make the fs unable to be degraded
>>>>> mounted.
>>>>>
>>>>> But in fact, some times all single chunks may be in the existing
>>>>> device and in that case, we should allow it to be rw degraded mounted.
>>>>>
>>>>> Such case can be easily reproduced using the following script:
>>>>>   # mkfs.btrfs -f -m raid1 -d sing /dev/sdb /dev/sdc
>>>>>   # wipefs -f /dev/sdc
>>>>>   # mount /dev/sdb -o degraded,rw
>>>>>
>>>>> If using btrfs-debug-tree to check /dev/sdb, one should find that the
>>>>> data chunk is only in sdb, so in fact it should allow degraded mount.
>>>>>
>>>>> This patchset will introduce a new per-chunk degradable check for
>>>>> btrfs, allow above case to succeed, and it's quite small anyway.
>>>>>
>>>>> And enhance kernel error message for missing device, at least kernel
>>>>> can know what's making mount failed, other than meaningless
>>>>> "failed to read system chunk/chunk tree -5".
>>>>
>>>> I'd like to get this merged to 4.14. The flush bio changes are now 
>>>> done,
>>>> so the base code should be stable. I've read the previous iterations of
>>>> this patchset, the comments and user feedback. The usecase coverage
>>>> seems to be good and what users expect.
>>>
>>> Thank you for the kindly remind.
>>>
>>>>
>>>> There are some bits in the implementation that I do not like, eg.
>>>> reintroducing memory allocation failure to the barrier check, but IIRC
>>>> no fundamental problems. Please refresh the patchset on top of current
>>>> code that's going to 4.13 (equvalent to the current for-next), I'll
>>>> review that and comment. One or more iterations might be needed, but
>>>> 4.14 target is within reach.
>>>
>>> I'll check the new flush infrastructure and figure out if we can 
>>> avoid re-introducing such memory allocation failure with the new 
>>> infrastructure.
>>
>>   As this is going to address the raid1 availability issue, its better to
>>   mark this for the stable. IMO. But I wonder if there is any objection ?
> 
> Not sure if stable maintainers (even normal subsystem maintainers) will 
> like it, as it's quite a large modification, including dev flush 
> infrastructure.
> 
> But since v4.14 will be an LTS kernel, we don't need to rush too much to 
> push this feature to stable, as long as the feature is planned to reach 
> v4.14.
I would personally tend to disagree.  It fixes a pretty severe data loss 
bug that arises from what most people for some reason think is 
acceptable normal usage.  I can understand not going too far back, but I 
do think it should probably be marked for at least 4.9.