Re: [PATCH] btrfs: auto enable discard=async when possible

[David Sterba <dsterba@suse.cz>]

On Wed, Jul 27, 2022 at 11:47:58PM +0500, Roman Mamedov wrote:
> On Wed, 27 Jul 2022 17:01:58 +0200
> David Sterba <dsterba@suse.com> wrote:
> 
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -1503,6 +1503,7 @@ enum {
> >  	BTRFS_MOUNT_DISCARD_ASYNC		= (1UL << 28),
> >  	BTRFS_MOUNT_IGNOREBADROOTS		= (1UL << 29),
> >  	BTRFS_MOUNT_IGNOREDATACSUMS		= (1UL << 30),
> > +	BTRFS_MOUNT_NODISCARD			= (1UL << 31),
> >  };
> >  
> >  #define BTRFS_DEFAULT_COMMIT_INTERVAL	(30)
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 3fac429cf8a4..8f8e33219d4d 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -3767,6 +3767,20 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> >  		btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations");
> >  	}
> >  
> > +	/*
> > +	 * For devices supporting discard turn on discard=async automatically,
> > +	 * unless it's already set or disabled. This could be turned off by
> > +	 * nodiscard for the same mount.
> > +	 */
> > +	if (!(btrfs_test_opt(fs_info, DISCARD_SYNC) ||
> > +	      btrfs_test_opt(fs_info, DISCARD_ASYNC) ||
> > +	      btrfs_test_opt(fs_info, NODISCARD)) &&
> > +	    fs_info->fs_devices->discardable) {
> > +		btrfs_set_and_info(fs_info, DISCARD_ASYNC,
> > +				"auto enabling discard=async");
> > +	      btrfs_clear_opt(fs_info->mount_opt, NODISCARD);
> 
> Spaces are used in the above line instead of a 2nd TAB.
> 
> Also I am probably clueless, but it seems the condition just checked that
> NODISCARD was not set, so what is the purpose of also clearing it?

I think it's out of habit and consistency with other option handling to
make it clear what's the new state (compression and datacow/datasum), but
you're right that here it's a bit redundant.