[v2] btrfs: drop the nossd flag when remounting with -o ssd
diff mbox

Message ID 20170331200850.7067-1-kilobyte@angband.pl
State Accepted
Headers show

Commit Message

Adam Borowski March 31, 2017, 8:08 p.m. UTC
And when turning on nossd, drop ssd_spread.

Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
On Fri, Mar 31, 2017 at 07:10:16PM +0200, David Sterba wrote:
> On Fri, Mar 31, 2017 at 06:00:08PM +0200, Hans van Kranenburg wrote:
> > On 03/31/2017 05:19 PM, Adam Borowski wrote:
> > > Not sure if setting NOSSD should also disable SSD_SPREAD, there's currently
> > > no way to disable that option once set.
> 
> Missing inverse of ssd_spread is probably unintentional, as we once
> added all complementary no* options, this one was forgotten.
> 
> And yes, nossd should turn off ssd and ssd_spread, as ssd_spread without
> ssd does not nothing anyway.

Added that.

> > How did you test this?
> > 
> > This was also my first thought, but here's a weird thing:
> > 
> > -# mount -o nossd /dev/sdx /mnt/btrfs/
> > 
> > BTRFS info (device sdx): not using ssd allocation scheme
> > 
> > -# mount -o remount,ssd /mnt/btrfs/
> > 
> > BTRFS info (device sdx): use ssd allocation scheme
> > 
> > -# mount -o remount,nossd /mnt/btrfs/
> > 
> > BTRFS info (device sdx): use ssd allocation scheme
> > 
> > That means that the case Opt_nossd: is never reached when doing this?

Seems to work for me:

[/tmp]# mount -onoatime foo /mnt/vol1 
[  619.436745] BTRFS: device fsid 954fd6c3-b3ce-4355-b79a-60ece7a6a4e0 devid 1 transid 5 /dev/loop0
[  619.438625] BTRFS info (device loop0): disk space caching is enabled
[  619.438627] BTRFS info (device loop0): has skinny extents
[  619.438629] BTRFS info (device loop0): flagging fs with big metadata feature
[  619.441989] BTRFS info (device loop0): creating UUID tree
[/tmp]# mount -oremount,ssd /mnt/vol1
[  629.755584] BTRFS info (device loop0): use ssd allocation scheme
[  629.755589] BTRFS info (device loop0): disk space caching is enabled
[/tmp]# mount -oremount,nossd /mnt/vol1
[  633.675867] BTRFS info (device loop0): not using ssd allocation scheme
[  633.675872] BTRFS info (device loop0): disk space caching is enabled

> > The fact that nossd,ssd,ssd_spread are different options complicates the
> > whole thing, compared to e.g. autodefrag, noautodefrag.
> 
> I think the the ssd flags reflect the autodetection of ssd, unlike
> autodefrag and others.

The autodetection works for /dev/sd* and /dev/mmcblk*, but not for most
other devices.

Two examples:
nbd to a piece of rotating rust says:
[45697.575192] BTRFS info (device nbd0): detected SSD devices, enabling SSD mode
loop on tmpfs (and in case it spills, all swap is on ssd):
claims it's rotational

> The ssd options says "enable the ssd mode", but it could be also
> auto-detected if the non-rotational device is detected.
> 
> nossd says, "do not do the autodetection, even if it's a non-rot
> device, also disable all ssd modes".

These two options are nice whenever the autodetection goes wrong.

> So Adam's patch needs to be updated so NOSSD also disables SSD_SPREAD.

M'kay, updated this patch.

> Adding the 'nossd_spread' would be good to have, even if it might be
> just a marginal usecase.

Not sure if there's much point.  In any case, that's a separate patch.
Should I add one while we're here?


Meow!

 fs/btrfs/super.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Hans van Kranenburg March 31, 2017, 8:24 p.m. UTC | #1
On 03/31/2017 10:08 PM, Adam Borowski wrote:
> And when turning on nossd, drop ssd_spread.
> 
> Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
> On Fri, Mar 31, 2017 at 07:10:16PM +0200, David Sterba wrote:
>> On Fri, Mar 31, 2017 at 06:00:08PM +0200, Hans van Kranenburg wrote:
>>> On 03/31/2017 05:19 PM, Adam Borowski wrote:
>>>> Not sure if setting NOSSD should also disable SSD_SPREAD, there's currently
>>>> no way to disable that option once set.
>>
>> Missing inverse of ssd_spread is probably unintentional, as we once
>> added all complementary no* options, this one was forgotten.
>>
>> And yes, nossd should turn off ssd and ssd_spread, as ssd_spread without
>> ssd does not nothing anyway.
> 
> Added that.
> 
>>> How did you test this?
>>>
>>> This was also my first thought, but here's a weird thing:
>>>
>>> -# mount -o nossd /dev/sdx /mnt/btrfs/
>>>
>>> BTRFS info (device sdx): not using ssd allocation scheme
>>>
>>> -# mount -o remount,ssd /mnt/btrfs/
>>>
>>> BTRFS info (device sdx): use ssd allocation scheme
>>>
>>> -# mount -o remount,nossd /mnt/btrfs/
>>>
>>> BTRFS info (device sdx): use ssd allocation scheme
>>>
>>> That means that the case Opt_nossd: is never reached when doing this?
> 
> Seems to work for me:
> 
> [/tmp]# mount -onoatime foo /mnt/vol1 
> [  619.436745] BTRFS: device fsid 954fd6c3-b3ce-4355-b79a-60ece7a6a4e0 devid 1 transid 5 /dev/loop0
> [  619.438625] BTRFS info (device loop0): disk space caching is enabled
> [  619.438627] BTRFS info (device loop0): has skinny extents
> [  619.438629] BTRFS info (device loop0): flagging fs with big metadata feature
> [  619.441989] BTRFS info (device loop0): creating UUID tree
> [/tmp]# mount -oremount,ssd /mnt/vol1
> [  629.755584] BTRFS info (device loop0): use ssd allocation scheme
> [  629.755589] BTRFS info (device loop0): disk space caching is enabled
> [/tmp]# mount -oremount,nossd /mnt/vol1
> [  633.675867] BTRFS info (device loop0): not using ssd allocation scheme
> [  633.675872] BTRFS info (device loop0): disk space caching is enabled

Yes, but we're not doing the same thing here.

You have a file via a loop mount. If I do that, I get the same output as
you show, the right messages when I remount ssd and nossd.

My test was lvm based on an ssd. When I mount that, I get the "detected
SSD devices, enabling SSD mode", and everytime I remount, being it ssd
or nossd, it *always* says "use ssd allocation scheme".

So, this needs some more research I guess. It doesn't feel right.

>>> The fact that nossd,ssd,ssd_spread are different options complicates the
>>> whole thing, compared to e.g. autodefrag, noautodefrag.
>>
>> I think the the ssd flags reflect the autodetection of ssd, unlike
>> autodefrag and others.
> 
> The autodetection works for /dev/sd* and /dev/mmcblk*, but not for most
> other devices.
> 
> Two examples:
> nbd to a piece of rotating rust says:
> [45697.575192] BTRFS info (device nbd0): detected SSD devices, enabling SSD mode
> loop on tmpfs (and in case it spills, all swap is on ssd):
> claims it's rotational
> 
>> The ssd options says "enable the ssd mode", but it could be also
>> auto-detected if the non-rotational device is detected.
>>
>> nossd says, "do not do the autodetection, even if it's a non-rot
>> device, also disable all ssd modes".
> 
> These two options are nice whenever the autodetection goes wrong.
> 
>> So Adam's patch needs to be updated so NOSSD also disables SSD_SPREAD.

Ack.

> M'kay, updated this patch.
> 
>> Adding the 'nossd_spread' would be good to have, even if it might be
>> just a marginal usecase.

Please no, don't make it more complex if not needed.

> Not sure if there's much point.  In any case, that's a separate patch.
> Should I add one while we're here?

Since the whole ssd thing is a bit of a joke actually, I'd rather see it
replaces with an option to choose an extent allocator algorithm.

The amount of if statements using this SSD things in btrfs in the kernel
can be counted on one hand, and what they actually do is quite
questionable (food for another mail thread).

> 
> Meow!
> 
>  fs/btrfs/super.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 06bd9b332e18..ac1ca22d0c34 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -549,16 +549,19 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>  		case Opt_ssd:
>  			btrfs_set_and_info(info, SSD,
>  					   "use ssd allocation scheme");
> +			btrfs_clear_opt(info->mount_opt, NOSSD);
>  			break;
>  		case Opt_ssd_spread:
>  			btrfs_set_and_info(info, SSD_SPREAD,
>  					   "use spread ssd allocation scheme");
>  			btrfs_set_opt(info->mount_opt, SSD);
> +			btrfs_clear_opt(info->mount_opt, NOSSD);
>  			break;
>  		case Opt_nossd:
>  			btrfs_set_and_info(info, NOSSD,
>  					     "not using ssd allocation scheme");
>  			btrfs_clear_opt(info->mount_opt, SSD);
> +			btrfs_clear_opt(info->mount_opt, SSD_SPREAD);
>  			break;
>  		case Opt_barrier:
>  			btrfs_clear_and_info(info, NOBARRIER,
>
Adam Borowski March 31, 2017, 8:43 p.m. UTC | #2
On Fri, Mar 31, 2017 at 10:24:57PM +0200, Hans van Kranenburg wrote:
> >>> How did you test this?
> >>>
> >>> This was also my first thought, but here's a weird thing:
> >>>
> >>> -# mount -o nossd /dev/sdx /mnt/btrfs/
> >>>
> >>> BTRFS info (device sdx): not using ssd allocation scheme
> >>>
> >>> -# mount -o remount,ssd /mnt/btrfs/
> >>>
> >>> BTRFS info (device sdx): use ssd allocation scheme
> >>>
> >>> -# mount -o remount,nossd /mnt/btrfs/
> >>>
> >>> BTRFS info (device sdx): use ssd allocation scheme
> >>>
> >>> That means that the case Opt_nossd: is never reached when doing this?
> > 
> > Seems to work for me:
> > 
> > [/tmp]# mount -onoatime foo /mnt/vol1 
> > [  619.436745] BTRFS: device fsid 954fd6c3-b3ce-4355-b79a-60ece7a6a4e0 devid 1 transid 5 /dev/loop0
> > [  619.438625] BTRFS info (device loop0): disk space caching is enabled
> > [  619.438627] BTRFS info (device loop0): has skinny extents
> > [  619.438629] BTRFS info (device loop0): flagging fs with big metadata feature
> > [  619.441989] BTRFS info (device loop0): creating UUID tree
> > [/tmp]# mount -oremount,ssd /mnt/vol1
> > [  629.755584] BTRFS info (device loop0): use ssd allocation scheme
> > [  629.755589] BTRFS info (device loop0): disk space caching is enabled
> > [/tmp]# mount -oremount,nossd /mnt/vol1
> > [  633.675867] BTRFS info (device loop0): not using ssd allocation scheme
> > [  633.675872] BTRFS info (device loop0): disk space caching is enabled
> 
> Yes, but we're not doing the same thing here.
> 
> You have a file via a loop mount. If I do that, I get the same output as
> you show, the right messages when I remount ssd and nossd.
> 
> My test was lvm based on an ssd. When I mount that, I get the "detected
> SSD devices, enabling SSD mode", and everytime I remount, being it ssd
> or nossd, it *always* says "use ssd allocation scheme".
> 
> So, this needs some more research I guess. It doesn't feel right.

I can't reproduce:

[~]# cat /proc/swaps
Filename				Type		Size	Used	Priority
/dev/sda2                               partition	8822780	0	-1
[~]# swapoff /dev/sda2
[~]# mkfs.btrfs -f /dev/sda2
...
[ 2459.856819] BTRFS info (device sda2): detected SSD devices, enabling SSD mode
[ 2459.857699] BTRFS info (device sda2): creating UUID tree
[ 2477.234868] BTRFS info (device sda2): not using ssd allocation scheme
[ 2477.234873] BTRFS info (device sda2): disk space caching is enabled
[ 2482.306649] BTRFS info (device sda2): use ssd allocation scheme
[ 2482.306654] BTRFS info (device sda2): disk space caching is enabled
[ 2483.618578] BTRFS info (device sda2): not using ssd allocation scheme
[ 2483.618583] BTRFS info (device sda2): disk space caching is enabled

Same partition on lvm:
[ 2813.259749] BTRFS info (device dm-0): detected SSD devices, enabling SSD mode
[ 2813.260586] BTRFS info (device dm-0): creating UUID tree
[ 2827.131076] BTRFS info (device dm-0): not using ssd allocation scheme
[ 2827.131081] BTRFS info (device dm-0): disk space caching is enabled
[ 2828.618841] BTRFS info (device dm-0): use ssd allocation scheme
[ 2828.618845] BTRFS info (device dm-0): disk space caching is enabled
[ 2829.546796] BTRFS info (device dm-0): not using ssd allocation scheme
[ 2829.546801] BTRFS info (device dm-0): disk space caching is enabled
[ 2833.770787] BTRFS info (device dm-0): use ssd allocation scheme
[ 2833.770792] BTRFS info (device dm-0): disk space caching is enabled

Seems to flip back and forth correctly for me.

Are you sure you have this patch applied?

> >> Adding the 'nossd_spread' would be good to have, even if it might be
> >> just a marginal usecase.
> 
> Please no, don't make it more complex if not needed.
> 
> > Not sure if there's much point.  In any case, that's a separate patch.
> > Should I add one while we're here?
> 
> Since the whole ssd thing is a bit of a joke actually, I'd rather see it
> replaces with an option to choose an extent allocator algorithm.
> 
> The amount of if statements using this SSD things in btrfs in the kernel
> can be counted on one hand, and what they actually do is quite
> questionable (food for another mail thread).

Ok, let's fix only existing options for now then.
Hans van Kranenburg March 31, 2017, 8:50 p.m. UTC | #3
On 03/31/2017 10:43 PM, Adam Borowski wrote:
> On Fri, Mar 31, 2017 at 10:24:57PM +0200, Hans van Kranenburg wrote:
>>
>> Yes, but we're not doing the same thing here.
>>
>> You have a file via a loop mount. If I do that, I get the same output as
>> you show, the right messages when I remount ssd and nossd.
>>
>> My test was lvm based on an ssd. When I mount that, I get the "detected
>> SSD devices, enabling SSD mode", and everytime I remount, being it ssd
>> or nossd, it *always* says "use ssd allocation scheme".
>>
>> So, this needs some more research I guess. It doesn't feel right.
> 
> I can't reproduce:
> 
> [~]# cat /proc/swaps
> Filename				Type		Size	Used	Priority
> /dev/sda2                               partition	8822780	0	-1
> [~]# swapoff /dev/sda2
> [~]# mkfs.btrfs -f /dev/sda2
> ...
> [ 2459.856819] BTRFS info (device sda2): detected SSD devices, enabling SSD mode
> [ 2459.857699] BTRFS info (device sda2): creating UUID tree
> [ 2477.234868] BTRFS info (device sda2): not using ssd allocation scheme
> [ 2477.234873] BTRFS info (device sda2): disk space caching is enabled
> [ 2482.306649] BTRFS info (device sda2): use ssd allocation scheme
> [ 2482.306654] BTRFS info (device sda2): disk space caching is enabled
> [ 2483.618578] BTRFS info (device sda2): not using ssd allocation scheme
> [ 2483.618583] BTRFS info (device sda2): disk space caching is enabled
> 
> Same partition on lvm:
> [ 2813.259749] BTRFS info (device dm-0): detected SSD devices, enabling SSD mode
> [ 2813.260586] BTRFS info (device dm-0): creating UUID tree
> [ 2827.131076] BTRFS info (device dm-0): not using ssd allocation scheme
> [ 2827.131081] BTRFS info (device dm-0): disk space caching is enabled
> [ 2828.618841] BTRFS info (device dm-0): use ssd allocation scheme
> [ 2828.618845] BTRFS info (device dm-0): disk space caching is enabled
> [ 2829.546796] BTRFS info (device dm-0): not using ssd allocation scheme
> [ 2829.546801] BTRFS info (device dm-0): disk space caching is enabled
> [ 2833.770787] BTRFS info (device dm-0): use ssd allocation scheme
> [ 2833.770792] BTRFS info (device dm-0): disk space caching is enabled
> 
> Seems to flip back and forth correctly for me.
> 
> Are you sure you have this patch applied?

Oh ok, that's with the patch. The output I show is without the patch.

If it does my output without the patch instead and the right output with
it applied, then the puzzle pieces are in the right place again.
David Sterba April 3, 2017, 12:24 p.m. UTC | #4
On Fri, Mar 31, 2017 at 10:24:57PM +0200, Hans van Kranenburg wrote:
> >> Adding the 'nossd_spread' would be good to have, even if it might be
> >> just a marginal usecase.
> 
> Please no, don't make it more complex if not needed.

The only use is when ssd,ssd_spread are on and then I'd just want to
disable ssd_spread, without disabling ssd at the same time.

1. mount -o ssd,ssd_spread
2. mount -o remount,nossd_spread

compared to

1. mount -o ssd,ssd_spread
2. mount -o remount,nossd
3. mount -o remount,ssd

I'd vote for adding nossd_spread, as the 'no-' options are common and
otherwise disabling ssd_spread would be another usage exception.

> > Not sure if there's much point.  In any case, that's a separate patch.
> > Should I add one while we're here?
> 
> Since the whole ssd thing is a bit of a joke actually, I'd rather see it
> replaces with an option to choose an extent allocator algorithm.

Yeah, SSD is not the shiny new tech anymore, so we'd need something more
future proof.

> The amount of if statements using this SSD things in btrfs in the kernel
> can be counted on one hand, and what they actually do is quite
> questionable (food for another mail thread).

That's right, do you have suggestions for futher SSD optimizations?
Other than better block alignment, faster flushes and no seek penalty, I
don't see much else.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba April 3, 2017, 12:25 p.m. UTC | #5
On Fri, Mar 31, 2017 at 10:08:50PM +0200, Adam Borowski wrote:
> And when turning on nossd, drop ssd_spread.
> 
> Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>

I've folded the two patches and queued for 4.11. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans van Kranenburg April 3, 2017, 10:43 p.m. UTC | #6
On 04/03/2017 02:24 PM, David Sterba wrote:
> On Fri, Mar 31, 2017 at 10:24:57PM +0200, Hans van Kranenburg wrote:
>>>> Adding the 'nossd_spread' would be good to have, even if it might be
>>>> just a marginal usecase.
>>
>> Please no, don't make it more complex if not needed.
> 
> The only use is when ssd,ssd_spread are on and then I'd just want to
> disable ssd_spread, without disabling ssd at the same time.
> 
> 1. mount -o ssd,ssd_spread
> 2. mount -o remount,nossd_spread
> 
> compared to
> 
> 1. mount -o ssd,ssd_spread
> 2. mount -o remount,nossd
> 3. mount -o remount,ssd
> 
> I'd vote for adding nossd_spread, as the 'no-' options are common and
> otherwise disabling ssd_spread would be another usage exception.

Yes, 'nossd_spread' would intuitively be the thing to try to get rid of
'ssd_spread' on a mounted fs. But, nossd_spread is not a feature, just
like noautodefrag isn't. nossd *is* a feature, but also a remount
option... :o)

The mount manpage displays the values as a choice between 3 exclusive
options: ssd|nossd|ssd_spread

They're like an increasing level of magic that is being applied:
   nossd < ssd < ssd_spread

So, that documentation with the | makes me think: I have to choose
either one. But that's not how it behaves, since some of them can appear

But don't listen to me, I don't know what the best thing is.

>>> Not sure if there's much point.  In any case, that's a separate patch.
>>> Should I add one while we're here?
>>
>> Since the whole ssd thing is a bit of a joke actually, I'd rather see it
>> replaces with an option to choose an extent allocator algorithm.
> 
> Yeah, SSD is not the shiny new tech anymore, so we'd need something more
> future proof.
> 
>> The amount of if statements using this SSD things in btrfs in the kernel
>> can be counted on one hand, and what they actually do is quite
>> questionable (food for another mail thread).
> 
> That's right, do you have suggestions for futher SSD optimizations?
> Other than better block alignment, faster flushes and no seek penalty, I
> don't see much else.

Yes, I'd like to start a discussion about that, but not buried in this
thread, and it's not about SSDs, but about a larger filesystem than the
average desktop computer and trade-offs between free space fragmentation
(going ENOSPC when 30 of your 40TiB is in use...) and metadata write
amplification (smaller writes leading to more cow, and, "let's track
extent tree storage inside the extent tree", which causes huge
avalanches of writing and writing and writing metadata with nossd).

And currently it's the ssd options that are influencing this a bit. But,
it doesn't make sense to punish people with a slow rotating drive with
things like having to write 40GiB of metadata when you feed 1 GiB of
data to balance...

But, more about that later, otherwise this is going to look too much
like a rant.
David Sterba April 10, 2017, 5:35 p.m. UTC | #7
On Tue, Apr 04, 2017 at 12:43:57AM +0200, Hans van Kranenburg wrote:
> On 04/03/2017 02:24 PM, David Sterba wrote:
> > On Fri, Mar 31, 2017 at 10:24:57PM +0200, Hans van Kranenburg wrote:
> >>>> Adding the 'nossd_spread' would be good to have, even if it might be
> >>>> just a marginal usecase.
> >>
> >> Please no, don't make it more complex if not needed.
> > 
> > The only use is when ssd,ssd_spread are on and then I'd just want to
> > disable ssd_spread, without disabling ssd at the same time.
> > 
> > 1. mount -o ssd,ssd_spread
> > 2. mount -o remount,nossd_spread
> > 
> > compared to
> > 
> > 1. mount -o ssd,ssd_spread
> > 2. mount -o remount,nossd
> > 3. mount -o remount,ssd
> > 
> > I'd vote for adding nossd_spread, as the 'no-' options are common and
> > otherwise disabling ssd_spread would be another usage exception.
> 
> Yes, 'nossd_spread' would intuitively be the thing to try to get rid of
> 'ssd_spread' on a mounted fs. But, nossd_spread is not a feature, just
> like noautodefrag isn't. nossd *is* a feature, but also a remount
> option... :o)
> 
> The mount manpage displays the values as a choice between 3 exclusive
> options: ssd|nossd|ssd_spread
> 
> They're like an increasing level of magic that is being applied:
>    nossd < ssd < ssd_spread
> 
> So, that documentation with the | makes me think: I have to choose
> either one. But that's not how it behaves, since some of them can appear

The documentation groups options by functionality, the "|" makes it
indeed confusing as exclusive options in this case. The rendering is not
consistent, I have "|" and also ",".  I hope that the text should
explain more than could fit into the brief option list and I'd like to
avoid complicating the syntax.

> But don't listen to me, I don't know what the best thing is.

Neither do I, so we talk about various aspects and update code or
documentation if we can make it more clear. I appreciate your feedback.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 06bd9b332e18..ac1ca22d0c34 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -549,16 +549,19 @@  int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 		case Opt_ssd:
 			btrfs_set_and_info(info, SSD,
 					   "use ssd allocation scheme");
+			btrfs_clear_opt(info->mount_opt, NOSSD);
 			break;
 		case Opt_ssd_spread:
 			btrfs_set_and_info(info, SSD_SPREAD,
 					   "use spread ssd allocation scheme");
 			btrfs_set_opt(info->mount_opt, SSD);
+			btrfs_clear_opt(info->mount_opt, NOSSD);
 			break;
 		case Opt_nossd:
 			btrfs_set_and_info(info, NOSSD,
 					     "not using ssd allocation scheme");
 			btrfs_clear_opt(info->mount_opt, SSD);
+			btrfs_clear_opt(info->mount_opt, SSD_SPREAD);
 			break;
 		case Opt_barrier:
 			btrfs_clear_and_info(info, NOBARRIER,