Re: [PATCH] btrfs: properly enable async discard when switching from RO->RW

[David Sterba <dsterba@suse.cz>]

On Mon, Jun 05, 2023 at 12:03:15PM -0700, Chris Mason wrote:
> async discard uses the BTRFS_FS_DISCARD_RUNNING bit in the fs_info to force
> discards off when the filesystem has aborted or we're generally not able
> to run discards.  This gets flipped on when we're mounted rw, and also
> when we go from ro->rw.
> 
> Commit 63a7cb13071842 enabled async discard by default, and this meant
> mount -o ro /dev/xxx /yyy had async discards turned on.
> 
> Unfortunately, this meant our check in btrfs_remount_cleanup() would see
> that discards are already on:
> 
>     /* If we toggled discard async */
>     if (!btrfs_raw_test_opt(old_opts, DISCARD_ASYNC) &&
> 	btrfs_test_opt(fs_info, DISCARD_ASYNC))
> 	    btrfs_discard_resume(fs_info);
> 
> So, we'd never call btrfs_discard_resume() when remounting the root
> filesystem from ro->rw.
> 
> drgn shows this really nicely:
> 
> import os
> import sys
> 
> from drgn.helpers.linux.fs import path_lookup
> from drgn import NULL, Object, Type, cast
> 
> def btrfs_sb(sb):
>     return cast("struct btrfs_fs_info *", sb.s_fs_info)
> 
> if len(sys.argv) == 1:
>     path = "/"
> else:
>     path = sys.argv[1]
> 
> fs_info = cast("struct btrfs_fs_info *", path_lookup(prog, path).mnt.mnt_sb.s_fs_info)
> 
> BTRFS_FS_DISCARD_RUNNING = 1 << prog['BTRFS_FS_DISCARD_RUNNING']
> if fs_info.flags & BTRFS_FS_DISCARD_RUNNING:
>     print("discard running flag is on")
> else:
>     print("discard running flag is off")
> 
> [root]# mount | grep nvme
> /dev/nvme0n1p3 on / type btrfs
> (rw,relatime,compress-force=zstd:3,ssd,discard=async,space_cache=v2,subvolid=5,subvol=/)
> 
> [root]# ./discard_running.drgn
> discard running flag is off
> 
> [root]# mount -o remount,discard=sync /
> [root]# mount -o remount,discard=async /
> [root]# ./discard_running.drgn
> discard running flag is on
> 
> The fix used here is calling btrfs_discard_resume() when we're going
> from ro->rw.  It already checks to make sure the async discard flag is
> on, so it'll do the right thing.
> 
> Fixes: 63a7cb13071842 ("btrfs: auto enable discard=async when possible")
> 
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Chris Mason <clm@fb.com>

Added to misc-next, thanks.