On 2020/3/24 上午3:28, David Sterba wrote: > On Sun, Mar 22, 2020 at 08:42:20AM +0800, Qu Wenruo wrote: >> >> >> On 2020/3/22 上午1:45, David Sterba wrote: >>> On Sat, Mar 21, 2020 at 09:43:21AM +0800, Qu Wenruo wrote: >>>> >>>> >>>> On 2020/3/21 上午2:43, fdmanana@kernel.org wrote: >>>>> From: Filipe Manana >>>>> >>>>> We are incorrectly dropping the raid56 and raid1c34 incompat flags when >>>>> there are still raid56 and raid1c34 block groups, not when we do not any >>>>> of those anymore. The logic just got unintentionally broken after adding >>>>> the support for the raid1c34 modes. >>>>> >>>>> Fix this by clear the flags only if we do not have block groups with the >>>>> respective profiles. >>>>> >>>>> Fixes: 9c907446dce3 ("btrfs: drop incompat bit for raid1c34 after last block group is gone") >>>>> Signed-off-by: Filipe Manana >>>> >>>> The fix is OK. >>>> >>>> Reviewed-by: Qu Wenruo >>>> >>>> Just interesting do we really need to remove such flags? >>>> To me, keep the flag is completely sane. >>> >>> So you'd suggest to keep a flag for a feature that's not used on the >>> filesystem so it's not possible to mount the filesystem on an older >>> kernel? >>> >> If user is using this feature, they aren't expecting mounting it on >> older kernel either. > > Before we go in a loop throwing single statements, let me take a broader > look. > > First thing, the removal of incompat bit was asked for by users, Hugo is > as reporter in the commit 6d58a55a894e863. > > https://lore.kernel.org/linux-btrfs/20190610144806.GB21016@carfax.org.uk/ > > " We've had a couple of cases in the past where people have tried out > a new feature on a new kernel, then turned it off again and not been > able to go back to an earlier kernel. Particularly in this case, I can > see people being surprised at the trapdoor. "I don't have any RAID13C > on this filesystem: why can't I go back to 5.2?" > > That itself is a strong sign to me that there's a need or usecase or a > good idea. Though we have a lot of them, this one was simple to > implement and made sense to me. For the raid56 it's a simple check, > unlike for other features that would need to go through significant > portion of metadata. > > Booting older kernel might sound like, why would anybody want to do > that, but if the bit is there preventing boot/mount, then it's an > unnecessary complication. In rescue environmnents it's gold. > > Usability improvements tend to be boring from code POV but it is > something that users can observe and make use of. > Thanks for the full picture. That makes sense. Thanks, Qu