Re: Are the btrfs mount options inconsistent?

2018-08-21 Thread Qu Wenruo


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?

2018-08-21 Thread David Howells
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?

2018-08-21 Thread Qu Wenruo


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?

2018-08-21 Thread David Sterba
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?

2018-08-21 Thread Austin S. Hemmelgarn

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?

2018-08-21 Thread David Howells
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?

2018-08-20 Thread Qu Wenruo


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?

2018-08-20 Thread David Howells
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?

2018-08-20 Thread David Howells
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?

2018-08-16 Thread David Sterba
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?

2018-08-16 Thread David Howells
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