On 03/31/2017 07:10 PM, 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: >>> The opposite case was already handled right in the very next switch entry. >>> >>> Reported-by: Hans van Kranenburg <hans.van.kranenb...@mendix.com> >>> Signed-off-by: Adam Borowski <kilob...@angband.pl> >>> --- >>> 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.
ssd_spread is tested exactly one time, in free-space-cache.c in btrfs_find_space_cluster As far as I can quickly find out, that code is reached regardless of which other options are set. So, it *does* something anyway if ssd is not set. And I think it prevents all writes from being split into multiple extents. So, nossd,ssd_spread (no ssd) is right now something that you can end up with (when the patch here is not applied) when playing with -o remount. > >>> >>> fs/btrfs/super.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c >>> index 06bd9b332e18..7342399951ad 100644 >>> --- a/fs/btrfs/super.c >>> +++ b/fs/btrfs/super.c >>> @@ -549,11 +549,13 @@ 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, >> >> 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? >> >> And... what should be the result of doing: >> -# mount -o remount,nossd,ssd /mnt/btrfs/ >> >> I guess it should be that the last one in the sequence wins? > > The last one wins. > >> 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 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". > > The manual page is not entirely clear about that, I'll update it. > > So Adam's patch needs to be updated so NOSSD also disables SSD_SPREAD. > Adding the 'nossd_spread' would be good to have, even if it might be > just a marginal usecase. > -- Hans van Kranenburg -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
