Re: Are the btrfs mount options inconsistent?
On 2018/8/21 下午10:35, David Howells wrote: > David Sterba wrote: > >> - nothing: auto-detect non-rotating devices, enable SSD mount option in turn > > And it's right that this should only turn on SSD if *all* the devices are > non-rotating? If I didn't miss anything again, yes. It's btrfs_open_one_device() who updates fs_devices->rotating, and any device with rotating disk will set it to 1. So it needs all device to to non-rotating to meet !fs_info->fs_devices->rotating. Thanks, Qu > > David > signature.asc Description: OpenPGP digital signature
Re: Are the btrfs mount options inconsistent?
David Sterba wrote: > - nothing: auto-detect non-rotating devices, enable SSD mount option in turn And it's right that this should only turn on SSD if *all* the devices are non-rotating? David
Re: Are the btrfs mount options inconsistent?
On 2018/8/21 下午10:24, David Sterba wrote: > On Tue, Aug 21, 2018 at 02:43:35PM +0100, David Howells wrote: >> Qu Wenruo wrote: >> >>> But to be more clear, NOSSD shouldn't be a special case. >>> In fact currently NOSSD only affects whether we will output the message >>> "enabling ssd optimization", no real effect if I didn't miss anything. > > There is a real effect. > >> That's not quite true. In: >> >> if (!btrfs_test_opt(fs_info, NOSSD) && >> !fs_info->fs_devices->rotating) { >> btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations"); >> } >> >> the call to btrfs_set_and_info() will turn on SSD. Oh, I just missed the btrfs_set part. (even it's myself introduced that function). What a shame... Thanks, Qu >> >> What this seems to me is that, normally, SSD will be turned on automatically >> unless at least one of the devices is a rotating medium - but this appears to >> be explicitly suppressed by the NOSSD option. > > Right. So expected behaviour: > > - nothing: auto-detect non-rotating devices, enable SSD mount option in turn > - nossd: disable auto-detection of non-rotating devices > - ssd: enable SSD optimizations uconditionally > - ssd_spread: implies SSD and affects some allocator decisions regarding > new extent alignments > signature.asc Description: OpenPGP digital signature
Re: Are the btrfs mount options inconsistent?
On Tue, Aug 21, 2018 at 02:43:35PM +0100, David Howells wrote: > Qu Wenruo wrote: > > > But to be more clear, NOSSD shouldn't be a special case. > > In fact currently NOSSD only affects whether we will output the message > > "enabling ssd optimization", no real effect if I didn't miss anything. There is a real effect. > That's not quite true. In: > > if (!btrfs_test_opt(fs_info, NOSSD) && > !fs_info->fs_devices->rotating) { > btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations"); > } > > the call to btrfs_set_and_info() will turn on SSD. > > What this seems to me is that, normally, SSD will be turned on automatically > unless at least one of the devices is a rotating medium - but this appears to > be explicitly suppressed by the NOSSD option. Right. So expected behaviour: - nothing: auto-detect non-rotating devices, enable SSD mount option in turn - nossd: disable auto-detection of non-rotating devices - ssd: enable SSD optimizations uconditionally - ssd_spread: implies SSD and affects some allocator decisions regarding new extent alignments
Re: Are the btrfs mount options inconsistent?
On 2018-08-21 09:43, David Howells wrote: Qu Wenruo wrote: But to be more clear, NOSSD shouldn't be a special case. In fact currently NOSSD only affects whether we will output the message "enabling ssd optimization", no real effect if I didn't miss anything. That's not quite true. In: if (!btrfs_test_opt(fs_info, NOSSD) && !fs_info->fs_devices->rotating) { btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations"); } the call to btrfs_set_and_info() will turn on SSD. What this seems to me is that, normally, SSD will be turned on automatically unless at least one of the devices is a rotating medium - but this appears to be explicitly suppressed by the NOSSD option. That's my understanding too (though I may be wrong, I'm not an expert on C). If this _isn't_ what's happening, then it needs to be changed so it is, that's what the documentation has pretty much always said, and is therefore how people expect it to work (also, it needs to work because there needs to be an option other than poking around at sysfs attributes to disable this on non-rotational media where it's not want4ed).
Re: Are the btrfs mount options inconsistent?
Qu Wenruo wrote: > But to be more clear, NOSSD shouldn't be a special case. > In fact currently NOSSD only affects whether we will output the message > "enabling ssd optimization", no real effect if I didn't miss anything. That's not quite true. In: if (!btrfs_test_opt(fs_info, NOSSD) && !fs_info->fs_devices->rotating) { btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations"); } the call to btrfs_set_and_info() will turn on SSD. What this seems to me is that, normally, SSD will be turned on automatically unless at least one of the devices is a rotating medium - but this appears to be explicitly suppressed by the NOSSD option. David
Re: Are the btrfs mount options inconsistent?
On 2018/8/20 下午8:24, David Howells wrote: > David Sterba wrote: > >> No it's not. Compression needs the checksums so nodatasum should disable >> compression, which is missing as you found out. > > Thanks. > > Btw, do fs_info->mount_opt end up inscribed on disk as is? I don't see > anywhere this obviously happens. mount_opt should only reflects runtime mount options, and currently there is no way to store mount option on-disk. > > Can I get rid of BTRFS_MOUNT_NOSSD as it would appear to be superfluous with > BTRFS_MOUNT_SSD? As you already found. But to be more clear, NOSSD shouldn't be a special case. In fact currently NOSSD only affects whether we will output the message "enabling ssd optimization", no real effect if I didn't miss anything. So the the states should be only 3: nossd, ssd, ssd + ssd_spread. Thanks, Qu > > David > signature.asc Description: OpenPGP digital signature
Re: Are the btrfs mount options inconsistent?
David Howells wrote: > Can I get rid of BTRFS_MOUNT_NOSSD as it would appear to be superfluous with > BTRFS_MOUNT_SSD? Ah... I guess it's not quite superfluous: if (!btrfs_test_opt(fs_info, NOSSD) && !fs_info->fs_devices->rotating) { btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations"); } It appears to be more of a four-state thing: definitely no, maybe no, yes and yes+spread. David
Re: Are the btrfs mount options inconsistent?
David Sterba wrote: > No it's not. Compression needs the checksums so nodatasum should disable > compression, which is missing as you found out. Thanks. Btw, do fs_info->mount_opt end up inscribed on disk as is? I don't see anywhere this obviously happens. Can I get rid of BTRFS_MOUNT_NOSSD as it would appear to be superfluous with BTRFS_MOUNT_SSD? David
Re: Are the btrfs mount options inconsistent?
On Thu, Aug 16, 2018 at 12:01:25PM +0100, David Howells wrote: > I'm trying to convert btrfs to use the new mount API stuff and I'm finding it > hard to work out the relationships between some of the arguments, specifically > datacow, datasum and compress*. > > What I see is that enabling datasum implies enabling datacow and that > disabling datacow implies disabling datasum - which seems reasonable. > > However, selecting compression implies enabling datacow and datasum, and > disabling datacow, as one might expect, disables compression - but disabling > datasum does not. Is that correct? No it's not. Compression needs the checksums so nodatasum should disable compression, which is missing as you found out. This invalid combination also causes some problems during device replace, but this was fixed in 4.18, so the missing part are the mount options.
Are the btrfs mount options inconsistent?
Hi Chris, I'm trying to convert btrfs to use the new mount API stuff and I'm finding it hard to work out the relationships between some of the arguments, specifically datacow, datasum and compress*. What I see is that enabling datasum implies enabling datacow and that disabling datacow implies disabling datasum - which seems reasonable. However, selecting compression implies enabling datacow and datasum, and disabling datacow, as one might expect, disables compression - but disabling datasum does not. Is that correct? David