Re: [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev()

Jan Kara
2025-07-04T09:05:32Z

OK, now that you've made me read the changelog :) :

On Fri 04-07-25 10:12:29, Qu Wenruo wrote:
> Currently all the filesystems implementing the
> super_opearations::shutdown() callback can not afford losing a device.
^^^ operations

> Thus fs_bdev_mark_dead() will just call the shutdown() callback for the
> involved filesystem.
>
> But it will no longer be the case, with multi-device filesystems like
> btrfs and bcachefs the filesystem can handle certain device loss without
> shutting down the whole filesystem.
>
> To allow those multi-device filesystems to be integrated to use
> fs_holder_ops:
>
> - Replace super_opearation::shutdown() with
> super_opearations::remove_bdev()
^^ again typos in work "operations"

> To better describe when the callback is called.
>
> - Add a new @bdev parameter to remove_bdev() callback
> To allow the fs to determine which device is missing, and do the
> proper handling when needed.
>
> For the existing shutdown callback users, the change is minimal.
> They only need to follow the rename and the new parameter list.
> The new @bdev parameter can be ignored if the filesystem can not afford
> losing any device, and continue using the old shutdown behavior.
>
> Btrfs is going to implement the callback soon, which will either
> shutdown the fs or continue read-write operations.
>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-ext4@vger.kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Cc: ntfs3@lists.linux.dev
> Cc: linux-xfs@vger.kernel.org
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Still feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> fs/exfat/super.c | 4 ++--
> fs/ext4/super.c | 4 ++--
> fs/f2fs/super.c | 4 ++--
> fs/ntfs3/super.c | 6 +++---
> fs/super.c | 4 ++--
> fs/xfs/xfs_super.c | 7 ++++---
> include/linux/fs.h | 7 ++++++-
> 7 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 7ed858937d45..a0e11166b194 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -172,7 +172,7 @@ int exfat_force_shutdown(struct super_block *sb, u32 flags)
> return 0;
> }
>
> -static void exfat_shutdown(struct super_block *sb)
> +static void exfat_remove_bdev(struct super_block *sb, struct block_device *bdev)
> {
> exfat_force_shutdown(sb, EXFAT_GOING_DOWN_NOSYNC);
> }
> @@ -202,7 +202,7 @@ static const struct super_operations exfat_sops = {
> .put_super = exfat_put_super,
> .statfs = exfat_statfs,
> .show_options = exfat_show_options,
> - .shutdown = exfat_shutdown,
> + .remove_bdev = exfat_remove_bdev,
> };
>
> enum {
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c7d39da7e733..d75b416401ae 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1456,7 +1456,7 @@ static void ext4_destroy_inode(struct inode *inode)
> EXT4_I(inode)->i_reserved_data_blocks);
> }
>
> -static void ext4_shutdown(struct super_block *sb)
> +static void ext4_remove_bdev(struct super_block *sb, struct block_device *bdev)
> {
> ext4_force_shutdown(sb, EXT4_GOING_FLAGS_NOLOGFLUSH);
> }
> @@ -1620,7 +1620,7 @@ static const struct super_operations ext4_sops = {
> .unfreeze_fs = ext4_unfreeze,
> .statfs = ext4_statfs,
> .show_options = ext4_show_options,
> - .shutdown = ext4_shutdown,
> + .remove_bdev = ext4_remove_bdev,
> #ifdef CONFIG_QUOTA
> .quota_read = ext4_quota_read,
> .quota_write = ext4_quota_write,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index bbf1dad6843f..8667af9f76e4 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2640,7 +2640,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> return err;
> }
>
> -static void f2fs_shutdown(struct super_block *sb)
> +static void f2fs_remove_bdev(struct super_block *sb, struct block_device *bdev)
> {
> f2fs_do_shutdown(F2FS_SB(sb), F2FS_GOING_DOWN_NOSYNC, false, false);
> }
> @@ -3264,7 +3264,7 @@ static const struct super_operations f2fs_sops = {
> .unfreeze_fs = f2fs_unfreeze,
> .statfs = f2fs_statfs,
> .remount_fs = f2fs_remount,
> - .shutdown = f2fs_shutdown,
> + .remove_bdev = f2fs_remove_bdev,
> };
>
> #ifdef CONFIG_FS_ENCRYPTION
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 920a1ab47b63..3e69dc805e3a 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -762,9 +762,9 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
> }
>
> /*
> - * ntfs_shutdown - super_operations::shutdown
> + * ntfs_remove_bdev - super_operations::remove_bdev
> */
> -static void ntfs_shutdown(struct super_block *sb)
> +static void ntfs_remove_bdev(struct super_block *sb, struct block_device *bdev)
> {
> set_bit(NTFS_FLAGS_SHUTDOWN_BIT, &ntfs_sb(sb)->flags);
> }
> @@ -821,7 +821,7 @@ static const struct super_operations ntfs_sops = {
> .put_super = ntfs_put_super,
> .statfs = ntfs_statfs,
> .show_options = ntfs_show_options,
> - .shutdown = ntfs_shutdown,
> + .remove_bdev = ntfs_remove_bdev,
> .sync_fs = ntfs_sync_fs,
> .write_inode = ntfs3_write_inode,
> };
> diff --git a/fs/super.c b/fs/super.c
> index 80418ca8e215..c972efb38f6a 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1463,8 +1463,8 @@ static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
> sync_filesystem(sb);
> shrink_dcache_sb(sb);
> evict_inodes(sb);
> - if (sb->s_op->shutdown)
> - sb->s_op->shutdown(sb);
> + if (sb->s_op->remove_bdev)
> + sb->s_op->remove_bdev(sb, bdev);
>
> super_unlock_shared(sb);
> }
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 0bc4b5489078..8e307b036133 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1276,8 +1276,9 @@ xfs_fs_free_cached_objects(
> }
>
> static void
> -xfs_fs_shutdown(
> - struct super_block *sb)
> +xfs_fs_remove_bdev(
> + struct super_block *sb,
> + struct block_device *bdev)
> {
> xfs_force_shutdown(XFS_M(sb), SHUTDOWN_DEVICE_REMOVED);
> }
> @@ -1308,7 +1309,7 @@ static const struct super_operations xfs_super_operations = {
> .show_options = xfs_fs_show_options,
> .nr_cached_objects = xfs_fs_nr_cached_objects,
> .free_cached_objects = xfs_fs_free_cached_objects,
> - .shutdown = xfs_fs_shutdown,
> + .remove_bdev = xfs_fs_remove_bdev,
> .show_stats = xfs_fs_show_stats,
> };
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b085f161ed22..b08af63d2d4f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2367,7 +2367,12 @@ struct super_operations {
> struct shrink_control *);
> long (*free_cached_objects)(struct super_block *,
> struct shrink_control *);
> - void (*shutdown)(struct super_block *sb);
> + /*
> + * Called when block device @bdev belonging to @sb is removed.
> + *
> + * If the fs can't afford the device loss, it should be shutdown.
> + */
> + void (*remove_bdev)(struct super_block *sb, struct block_device *bdev);
> };
>
> /*
> --
> 2.50.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR