[PATCH RFC 0/6] btrfs: go fs_holder_ops and add shutdown_bdev() callback

[PATCH RFC 0/6] btrfs: go fs_holder_ops and add shutdown_bdev() callback

[Qu Wenruo]

This series is relying on another series here:
https://lore.kernel.org/linux-btrfs/cover.1750137547.git.wqu@suse.com/

That above series prepare btrfs to use fs_holder_ops, and the first
patch in this series is exactly making btrfs use that fs_holder_ops.

Then patch 2~4 implements the shutdown ioctl for btrfs.

For now the shutdown ioctl has no proper sync yet.

Patch 5 is the one affecting the generic fs_holder_ops and
super_operations, that we need a shutdown_bdev() variant, which passes
the bdev to the fs and let the fs to determine if they can continue
operation or needs to shutdown.

AKA, the shutdown_bdev() variant is for multi-devices filesystems like
btrfs and bcachefs.

And finally implement the shutdown_bdev() for btrfs, so that eventually
generic/730 can properly pass.

Reason for RFC:

There are still test failures for shutdown group, mostly due to the fact
that btrfs is not implementing the proper flags handling (e.g. currently
the shutdown never sync the fs no matter what).

But I want to get some feedback about the new
super_operations::shutdown_bdev() call back before committing too much.

Qu Wenruo (6):
  btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN
  btrfs: reject file operations if in shutdown state
  btrfs: reject delalloc ranges if the fs is shutdown
  btrfs: implement shutdown ioctl
  fs: introduce a shutdown_bdev super block operation
  btrfs: implement shutdown_bdev super operation callback

 fs/btrfs/file.c            | 25 ++++++++++++++++++++++++-
 fs/btrfs/fs.h              | 18 ++++++++++++++++++
 fs/btrfs/inode.c           | 14 +++++++++++++-
 fs/btrfs/ioctl.c           | 21 +++++++++++++++++++++
 fs/btrfs/messages.c        |  1 +
 fs/btrfs/reflink.c         |  3 +++
 fs/btrfs/super.c           | 34 ++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.c         |  2 ++
 fs/btrfs/volumes.h         |  5 +++++
 fs/super.c                 |  4 +++-
 include/linux/fs.h         | 10 ++++++++++
 include/uapi/linux/btrfs.h |  9 +++++++++
 12 files changed, 143 insertions(+), 3 deletions(-)

-- 
2.49.0

[PATCH RFC 1/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN

[Qu Wenruo]

This is btrfs' equivalent of XFS_IOC_GOINGDOWN or EXT4_IOC_SHUTDOWN,
after entering the emergency shutdown state, all operations will return
error (-EIO), and can not be bring back to normal state until unmount.

We reuse the existing btrfs_handle_fs_error() function, which will mark
the fs read-only and output an error message.
This allows us to handle put_super() callback as usual, as the fs is
already marked as error and RO, thus we need no special handling.

The only extra thing is to set the EMERGENCY_SHUTDOWN flag to reject
future operations with -EIO.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/fs.h       | 18 ++++++++++++++++++
 fs/btrfs/messages.c |  1 +
 2 files changed, 19 insertions(+)

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index d90304d4e32c..8cc4468a9894 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -29,6 +29,7 @@
 #include "extent-io-tree.h"
 #include "async-thread.h"
 #include "block-rsv.h"
+#include "messages.h"
 
 struct inode;
 struct super_block;
@@ -120,6 +121,12 @@ enum {
 	/* No more delayed iput can be queued. */
 	BTRFS_FS_STATE_NO_DELAYED_IPUT,
 
+	/*
+	 * Emergency shutdown, a step further than trans aborted by rejecting
+	 * all operations.
+	 */
+	BTRFS_FS_STATE_EMERGENCY_SHUTDOWN,
+
 	BTRFS_FS_STATE_COUNT
 };
 
@@ -1100,6 +1107,17 @@ static inline void btrfs_wake_unfinished_drop(struct btrfs_fs_info *fs_info)
 	(unlikely(test_bit(BTRFS_FS_STATE_LOG_CLEANUP_ERROR,		\
 			   &(fs_info)->fs_state)))
 
+static inline bool btrfs_is_shutdown(struct btrfs_fs_info *fs_info)
+{
+	return test_bit(BTRFS_FS_STATE_EMERGENCY_SHUTDOWN, &fs_info->fs_state);
+}
+
+static inline void btrfs_shutdown(struct btrfs_fs_info *fs_info)
+{
+	btrfs_handle_fs_error(fs_info, -EIO, "filesystem emergency shutdown");
+	set_bit(BTRFS_FS_STATE_EMERGENCY_SHUTDOWN, &fs_info->fs_state);
+}
+
 /*
  * We use folio flag owner_2 to indicate there is an ordered extent with
  * unfinished IO.
diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 363fd28c0268..2bb4bcb7c2cd 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -23,6 +23,7 @@ static const char fs_state_chars[] = {
 	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
 	[BTRFS_FS_STATE_SKIP_META_CSUMS]	= 'S',
 	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= 'L',
+	[BTRFS_FS_STATE_EMERGENCY_SHUTDOWN]	= 'E',
 };
 
 static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
-- 
2.49.0

[PATCH RFC 2/6] btrfs: reject file operations if in shutdown state

[Qu Wenruo]

This includes the following callbacks of file_operations:

- read_iter()
- write_iter()
- mmap()
- open()
- remap_file_range()
- uring_cmd()
- splice_read()
  This requires a small wrapper to do the extra shutdown check, then call
  the regular filemap_splice_read() function

This should reject most of the file operations on a shutdown btrfs.

The callback ioctl() is intentionally skipped, as ext4 doesn't do the
shutdown check on ioctl() either, thus I believe there is some special
require for ioctl() callback even if the fs is fully shutdown.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/file.c    | 25 ++++++++++++++++++++++++-
 fs/btrfs/ioctl.c   |  3 +++
 fs/btrfs/reflink.c |  3 +++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 2902de88dd1b..4ebb96f9514e 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1420,6 +1420,8 @@ ssize_t btrfs_do_write_iter(struct kiocb *iocb, struct iov_iter *from,
 	struct btrfs_inode *inode = BTRFS_I(file_inode(file));
 	ssize_t num_written, num_sync;
 
+	if (unlikely(btrfs_is_shutdown(inode->root->fs_info)))
+		return -EIO;
 	/*
 	 * If the fs flips readonly due to some impossible error, although we
 	 * have opened a file as writable, we have to stop this write operation
@@ -1982,6 +1984,8 @@ static int btrfs_file_mmap(struct file	*filp, struct vm_area_struct *vma)
 {
 	struct address_space *mapping = filp->f_mapping;
 
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(file_inode(filp)))))
+		return -EIO;
 	if (!mapping->a_ops->read_folio)
 		return -ENOEXEC;
 
@@ -3041,6 +3045,9 @@ static long btrfs_fallocate(struct file *file, int mode,
 	int blocksize = BTRFS_I(inode)->root->fs_info->sectorsize;
 	int ret;
 
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(inode))))
+		return -EIO;
+
 	/* Do not allow fallocate in ZONED mode */
 	if (btrfs_is_zoned(inode_to_fs_info(inode)))
 		return -EOPNOTSUPP;
@@ -3732,6 +3739,9 @@ static int btrfs_file_open(struct inode *inode, struct file *filp)
 {
 	int ret;
 
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(inode))))
+		return -EIO;
+
 	filp->f_mode |= FMODE_NOWAIT | FMODE_CAN_ODIRECT;
 
 	ret = fsverity_file_open(inode, filp);
@@ -3744,6 +3754,9 @@ static ssize_t btrfs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	ssize_t ret = 0;
 
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(file_inode(iocb->ki_filp)))))
+		return -EIO;
+
 	if (iocb->ki_flags & IOCB_DIRECT) {
 		ret = btrfs_direct_read(iocb, to);
 		if (ret < 0 || !iov_iter_count(to) ||
@@ -3754,10 +3767,20 @@ static ssize_t btrfs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return filemap_read(iocb, to, ret);
 }
 
+static ssize_t btrfs_file_splice_read(struct file *in, loff_t *ppos,
+				      struct pipe_inode_info *pipe,
+				      size_t len, unsigned int flags)
+{
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(file_inode(in)))))
+		return -EIO;
+
+	return filemap_splice_read(in, ppos, pipe, len, flags);
+}
+
 const struct file_operations btrfs_file_operations = {
 	.llseek		= btrfs_file_llseek,
 	.read_iter      = btrfs_file_read_iter,
-	.splice_read	= filemap_splice_read,
+	.splice_read	= btrfs_file_splice_read,
 	.write_iter	= btrfs_file_write_iter,
 	.splice_write	= iter_file_splice_write,
 	.mmap		= btrfs_file_mmap,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 608a63f07e6b..84a516053a8e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5067,6 +5067,9 @@ static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issu
 
 int btrfs_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(file_inode(cmd->file)))))
+		return -EIO;
+
 	switch (cmd->cmd_op) {
 	case BTRFS_IOC_ENCODED_READ:
 #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 0197bd9160a7..123a5682514b 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -869,6 +869,9 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	bool same_inode = dst_inode == src_inode;
 	int ret;
 
+	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(file_inode(src_file)))))
+		return -EIO;
+
 	if (remap_flags & ~(REMAP_FILE_DEDUP | REMAP_FILE_ADVISORY))
 		return -EINVAL;
 
-- 
2.49.0

[PATCH RFC 3/6] btrfs: reject delalloc ranges if the fs is shutdown

[Qu Wenruo]

If the filesystem has dirty pages before the fs is shutdown, we should
no longer write them back, instead should treat them as writeback error.

Handle such situation by marking all those delalloc range as error and
let error handling path to clean them up.

For ranges that already have ordered extent created, let them continue
the writeback, and at ordered io finish time the file extent item update
will be rejected as the fs is already marked error.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 31feb470fe33..b8eb3153f5e0 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -862,6 +862,9 @@ static void compress_file_range(struct btrfs_work *work)
 	int compress_type = fs_info->compress_type;
 	int compress_level = fs_info->compress_level;
 
+	if (unlikely(btrfs_is_shutdown(fs_info)))
+		goto cleanup_and_bail_uncompressed;
+
 	inode_should_defrag(inode, start, end, end - start + 1, SZ_16K);
 
 	/*
@@ -1277,6 +1280,11 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
 	unsigned long page_ops;
 	int ret = 0;
 
+	if (unlikely(btrfs_is_shutdown(fs_info))) {
+		ret = -EIO;
+		goto out_unlock;
+	}
+
 	if (btrfs_is_free_space_inode(inode)) {
 		ret = -EINVAL;
 		goto out_unlock;
@@ -2027,7 +2035,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_root *root = inode->root;
-	struct btrfs_path *path;
+	struct btrfs_path *path = NULL;
 	u64 cow_start = (u64)-1;
 	/*
 	 * If not 0, represents the inclusive end of the last fallback_to_cow()
@@ -2047,6 +2055,10 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 	 */
 	ASSERT(!btrfs_is_zoned(fs_info) || btrfs_is_data_reloc_root(root));
 
+	if (unlikely(btrfs_is_shutdown(fs_info))) {
+		ret = -EIO;
+		goto error;
+	}
 	path = btrfs_alloc_path();
 	if (!path) {
 		ret = -ENOMEM;
-- 
2.49.0

[PATCH RFC 4/6] btrfs: implement shutdown ioctl

[Qu Wenruo]

The shutdown interface should all follow the XFS one, which use magic
'X', and ioctl number 125, with a u32 as flags.

For now btrfs doesn't follow the flags, as there is no traditional
journal in btrfs (the log tree is to mostly speed up fsync).

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ioctl.c           | 18 ++++++++++++++++++
 include/uapi/linux/btrfs.h |  9 +++++++++
 2 files changed, 27 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 84a516053a8e..1fd7486af851 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5213,6 +5213,16 @@ static int btrfs_ioctl_subvol_sync(struct btrfs_fs_info *fs_info, void __user *a
 	return 0;
 }
 
+static int btrfs_emergency_shutdown(struct btrfs_fs_info *fs_info, u32 flags)
+{
+	if (flags >= BTRFS_SHUTDOWN_FLAGS_LAST)
+		return -EINVAL;
+
+	/* For now, btrfs do not distinguish the different flags. */
+	btrfs_shutdown(fs_info);
+	return 0;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -5368,6 +5378,14 @@ long btrfs_ioctl(struct file *file, unsigned int
 #endif
 	case BTRFS_IOC_SUBVOL_SYNC_WAIT:
 		return btrfs_ioctl_subvol_sync(fs_info, argp);
+	case BTRFS_IOC_SHUTDOWN:
+		u32 flags;
+
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (get_user(flags, (__u32 __user *)arg))
+			return -EFAULT;
+		return btrfs_emergency_shutdown(fs_info, flags);
 	}
 
 	return -ENOTTY;
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index dd02160015b2..8f6324cf15d9 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -1096,6 +1096,12 @@ enum btrfs_err_code {
 	BTRFS_ERROR_DEV_RAID1C4_MIN_NOT_MET,
 };
 
+/* Flags for IOC_SHUTDOWN, should match XFS' flags. */
+#define BTRFS_SHUTDOWN_FLAGS_DEFAULT	0x0
+#define BTRFS_SHUTDOWN_FLAGS_LOGFLUSH	0x1
+#define BTRFS_SHUTDOWN_FLAGS_NOLOGFLUSH	0x2
+#define BTRFS_SHUTDOWN_FLAGS_LAST	0x3
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -1217,6 +1223,9 @@ enum btrfs_err_code {
 #define BTRFS_IOC_SUBVOL_SYNC_WAIT _IOW(BTRFS_IOCTL_MAGIC, 65, \
 					struct btrfs_ioctl_subvol_wait)
 
+/* Shutdown ioctl should follow XFS's interfaces, thus not using btrfs magic. */
+#define BTRFS_IOC_SHUTDOWN	_IOR('X', 125, __u32)
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.49.0

[PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

Currently we already have the super_operations::shutdown() callback,
which is called when the block device of a filesystem is marked dead.

However this is mostly for single(ish) block device filesystems.

For multi-device filesystems, they may afford a missing device, thus may
continue work without fully shutdown the filesystem.

So add a new super_operation::shutdown_bdev() callback, for mutli-device
filesystems like btrfs and bcachefs.

For now the only user is fs_holder_ops::mark_dead(), which will call
shutdown_bdev() if supported.
If not supported then fallback to the original shutdown() callback.

Btrfs is going to add the usage of shutdown_bdev() soon.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/super.c         |  4 +++-
 include/linux/fs.h | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 21799e213fd7..8242a03bd5ce 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1461,7 +1461,9 @@ 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)
+	if (sb->s_op->shutdown_bdev)
+		sb->s_op->shutdown_bdev(sb, bdev);
+	else if (sb->s_op->shutdown)
 		sb->s_op->shutdown(sb);
 
 	super_unlock_shared(sb);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 96c7925a6551..4f6b4b3cbe22 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2363,7 +2363,17 @@ struct super_operations {
 				  struct shrink_control *);
 	long (*free_cached_objects)(struct super_block *,
 				    struct shrink_control *);
+	/*
+	 * For single-device filesystems. Called when the only block device is
+	 * marked dead.
+	 */
 	void (*shutdown)(struct super_block *sb);
+
+	/*
+	 * For multi-device filesystems. Called when any of its block device is
+	 * marked dead.
+	 */
+	void (*shutdown_bdev)(struct super_block *sb, struct block_device *bdev);
 };
 
 /*
-- 
2.49.0

[PATCH RFC 6/6] btrfs: implement shutdown_bdev super operation callback

[Qu Wenruo]

For this callback, btrfs will:

- Go degraded if the fs can still maintain RW operations

- Shutdown if the fs can not maintain RW operations

I know the shutdown can be a little overkilled, if one has a RAID1
metadata and RAID0 data, in that case one can still read data with 50%
rate to got some good data.

But it can also be as bad as the only device went missing for a single
device btrfs.

So here we go safe other than sorry when handling missing device.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/super.c   | 34 ++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.c |  2 ++
 fs/btrfs/volumes.h |  5 +++++
 3 files changed, 41 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 82ce0625b2f0..f82f9be41321 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2397,6 +2397,39 @@ static long btrfs_free_cached_objects(struct super_block *sb, struct shrink_cont
 	return 0;
 }
 
+static void btrfs_shutdown_bdev(struct super_block *sb, struct block_device *bdev)
+{
+	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
+	struct btrfs_device *device;
+	struct btrfs_dev_lookup_args lookup_args = { .devt = bdev->bd_dev };
+
+	mutex_lock(&fs_info->fs_devices->device_list_mutex);
+	device = btrfs_find_device(fs_info->fs_devices, &lookup_args);
+	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+	if (!device) {
+		btrfs_warn(fs_info, "unable to find btrfs device for block device '%pg'",
+			   bdev);
+		return;
+	}
+	set_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
+	device->fs_devices->missing_devices++;
+	if (test_and_clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
+		list_del_init(&device->dev_alloc_list);
+		device->fs_devices->rw_devices--;
+	}
+	if (!btrfs_check_rw_degradable(fs_info, device)) {
+		btrfs_warn_in_rcu(fs_info,
+	"btrfs device id %llu path %s has gone missing, can not maintain read-write",
+				  device->devid, btrfs_dev_name(device));
+		btrfs_shutdown(fs_info);
+		return;
+	}
+	btrfs_warn_in_rcu(fs_info,
+	"btrfs device id %llu path %s has gone missing, continue degraded",
+			  device->devid, btrfs_dev_name(device));
+	btrfs_set_opt(fs_info->mount_opt, DEGRADED);
+}
+
 static const struct super_operations btrfs_super_ops = {
 	.drop_inode	= btrfs_drop_inode,
 	.evict_inode	= btrfs_evict_inode,
@@ -2412,6 +2445,7 @@ static const struct super_operations btrfs_super_ops = {
 	.unfreeze_fs	= btrfs_unfreeze,
 	.nr_cached_objects = btrfs_nr_cached_objects,
 	.free_cached_objects = btrfs_free_cached_objects,
+	.shutdown_bdev	= btrfs_shutdown_bdev,
 };
 
 static const struct file_operations btrfs_ctl_fops = {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 541d17ca5dcf..5639df71ef91 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6792,6 +6792,8 @@ static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args,
 static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args,
 				  const struct btrfs_device *device)
 {
+	if (args->devt)
+		return device->devt == args->devt;
 	if (args->missing) {
 		if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state) &&
 		    !device->bdev)
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index afa71d315c46..2c5e3ebc5a2b 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -652,6 +652,11 @@ struct btrfs_dev_lookup_args {
 	u64 devid;
 	u8 *uuid;
 	u8 *fsid;
+	/*
+	 * If devt is specified, all other members will be ignored as it is
+	 * enough to uniquely locate a device.
+	 */
+	dev_t devt;
 	bool missing;
 };
 
-- 
2.49.0

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Jan Kara]

On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
> Currently we already have the super_operations::shutdown() callback,
> which is called when the block device of a filesystem is marked dead.
> 
> However this is mostly for single(ish) block device filesystems.
> 
> For multi-device filesystems, they may afford a missing device, thus may
> continue work without fully shutdown the filesystem.
> 
> So add a new super_operation::shutdown_bdev() callback, for mutli-device
> filesystems like btrfs and bcachefs.
> 
> For now the only user is fs_holder_ops::mark_dead(), which will call
> shutdown_bdev() if supported.
> If not supported then fallback to the original shutdown() callback.
> 
> Btrfs is going to add the usage of shutdown_bdev() soon.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Thanks for the patch. I think that we could actually add 'bdev' that
triggered shutdown among arguments ->shutdown takes instead of introducing
a new handler.

								Honza

> ---
>  fs/super.c         |  4 +++-
>  include/linux/fs.h | 10 ++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 21799e213fd7..8242a03bd5ce 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1461,7 +1461,9 @@ 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)
> +	if (sb->s_op->shutdown_bdev)
> +		sb->s_op->shutdown_bdev(sb, bdev);
> +	else if (sb->s_op->shutdown)
>  		sb->s_op->shutdown(sb);
>  
>  	super_unlock_shared(sb);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 96c7925a6551..4f6b4b3cbe22 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2363,7 +2363,17 @@ struct super_operations {
>  				  struct shrink_control *);
>  	long (*free_cached_objects)(struct super_block *,
>  				    struct shrink_control *);
> +	/*
> +	 * For single-device filesystems. Called when the only block device is
> +	 * marked dead.
> +	 */
>  	void (*shutdown)(struct super_block *sb);
> +
> +	/*
> +	 * For multi-device filesystems. Called when any of its block device is
> +	 * marked dead.
> +	 */
> +	void (*shutdown_bdev)(struct super_block *sb, struct block_device *bdev);
>  };
>  
>  /*
> -- 
> 2.49.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/21 01:06, Jan Kara 写道:
> On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
>> Currently we already have the super_operations::shutdown() callback,
>> which is called when the block device of a filesystem is marked dead.
>>
>> However this is mostly for single(ish) block device filesystems.
>>
>> For multi-device filesystems, they may afford a missing device, thus may
>> continue work without fully shutdown the filesystem.
>>
>> So add a new super_operation::shutdown_bdev() callback, for mutli-device
>> filesystems like btrfs and bcachefs.
>>
>> For now the only user is fs_holder_ops::mark_dead(), which will call
>> shutdown_bdev() if supported.
>> If not supported then fallback to the original shutdown() callback.
>>
>> Btrfs is going to add the usage of shutdown_bdev() soon.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Thanks for the patch. I think that we could actually add 'bdev' that
> triggered shutdown among arguments ->shutdown takes instead of introducing
> a new handler.
> 
> 								Honza

That makes sense.

Will add the new bdev parameters instead.

Thanks,
Qu

> 
>> ---
>>   fs/super.c         |  4 +++-
>>   include/linux/fs.h | 10 ++++++++++
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 21799e213fd7..8242a03bd5ce 100644
>> --- a/fs/super.c
>> +++ b/fs/super.c
>> @@ -1461,7 +1461,9 @@ 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)
>> +	if (sb->s_op->shutdown_bdev)
>> +		sb->s_op->shutdown_bdev(sb, bdev);
>> +	else if (sb->s_op->shutdown)
>>   		sb->s_op->shutdown(sb);
>>   
>>   	super_unlock_shared(sb);
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 96c7925a6551..4f6b4b3cbe22 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2363,7 +2363,17 @@ struct super_operations {
>>   				  struct shrink_control *);
>>   	long (*free_cached_objects)(struct super_block *,
>>   				    struct shrink_control *);
>> +	/*
>> +	 * For single-device filesystems. Called when the only block device is
>> +	 * marked dead.
>> +	 */
>>   	void (*shutdown)(struct super_block *sb);
>> +
>> +	/*
>> +	 * For multi-device filesystems. Called when any of its block device is
>> +	 * marked dead.
>> +	 */
>> +	void (*shutdown_bdev)(struct super_block *sb, struct block_device *bdev);
>>   };
>>   
>>   /*
>> -- 
>> 2.49.0
>>

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christoph Hellwig]

On Fri, Jun 20, 2025 at 05:36:52PM +0200, Jan Kara wrote:
> On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
> > Currently we already have the super_operations::shutdown() callback,
> > which is called when the block device of a filesystem is marked dead.
> > 
> > However this is mostly for single(ish) block device filesystems.
> > 
> > For multi-device filesystems, they may afford a missing device, thus may
> > continue work without fully shutdown the filesystem.
> > 
> > So add a new super_operation::shutdown_bdev() callback, for mutli-device
> > filesystems like btrfs and bcachefs.
> > 
> > For now the only user is fs_holder_ops::mark_dead(), which will call
> > shutdown_bdev() if supported.
> > If not supported then fallback to the original shutdown() callback.
> > 
> > Btrfs is going to add the usage of shutdown_bdev() soon.
> > 
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Thanks for the patch. I think that we could actually add 'bdev' that
> triggered shutdown among arguments ->shutdown takes instead of introducing
> a new handler.

I don't really think that's a good idea as-is.  The current ->shutdown
callback is called ->shutdown because it is expected to shut the file
system down.  That's why I suggested to Qu to add a new devloss callback,
to describe that a device is lost.  In a file system with built-in
redundancy that is not a shutdown.  So Qu, please add a devloss
callback.  And maybe if we have no other good use for the shutdown
callback we can remove it in favor of the devloss one.  But having
something named shutdown take the block device and not always shutting
the file system down is highly confusing.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/23 14:45, Christoph Hellwig 写道:
> On Fri, Jun 20, 2025 at 05:36:52PM +0200, Jan Kara wrote:
>> On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
>>> Currently we already have the super_operations::shutdown() callback,
>>> which is called when the block device of a filesystem is marked dead.
>>>
>>> However this is mostly for single(ish) block device filesystems.
>>>
>>> For multi-device filesystems, they may afford a missing device, thus may
>>> continue work without fully shutdown the filesystem.
>>>
>>> So add a new super_operation::shutdown_bdev() callback, for mutli-device
>>> filesystems like btrfs and bcachefs.
>>>
>>> For now the only user is fs_holder_ops::mark_dead(), which will call
>>> shutdown_bdev() if supported.
>>> If not supported then fallback to the original shutdown() callback.
>>>
>>> Btrfs is going to add the usage of shutdown_bdev() soon.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>
>> Thanks for the patch. I think that we could actually add 'bdev' that
>> triggered shutdown among arguments ->shutdown takes instead of introducing
>> a new handler.
> 
> I don't really think that's a good idea as-is.  The current ->shutdown
> callback is called ->shutdown because it is expected to shut the file
> system down.  That's why I suggested to Qu to add a new devloss callback,
> to describe that a device is lost.  In a file system with built-in
> redundancy that is not a shutdown.  So Qu, please add a devloss
> callback.  And maybe if we have no other good use for the shutdown
> callback we can remove it in favor of the devloss one.  But having
> something named shutdown take the block device and not always shutting
> the file system down is highly confusing.

OK, I got the point of the name "devloss" now, didn't notice the naming 
itself is important at that timing.

And in fact a new callback is much easier on me, no need to modify the 
code of other fses.

@Jan, would this be acceptable for a new devloss() callback instead?

Thanks,
Qu

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christian Brauner]

On Sun, Jun 22, 2025 at 10:15:19PM -0700, Christoph Hellwig wrote:
> On Fri, Jun 20, 2025 at 05:36:52PM +0200, Jan Kara wrote:
> > On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
> > > Currently we already have the super_operations::shutdown() callback,
> > > which is called when the block device of a filesystem is marked dead.
> > > 
> > > However this is mostly for single(ish) block device filesystems.
> > > 
> > > For multi-device filesystems, they may afford a missing device, thus may
> > > continue work without fully shutdown the filesystem.
> > > 
> > > So add a new super_operation::shutdown_bdev() callback, for mutli-device
> > > filesystems like btrfs and bcachefs.
> > > 
> > > For now the only user is fs_holder_ops::mark_dead(), which will call
> > > shutdown_bdev() if supported.
> > > If not supported then fallback to the original shutdown() callback.
> > > 
> > > Btrfs is going to add the usage of shutdown_bdev() soon.
> > > 
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > 
> > Thanks for the patch. I think that we could actually add 'bdev' that
> > triggered shutdown among arguments ->shutdown takes instead of introducing
> > a new handler.
> 
> I don't really think that's a good idea as-is.  The current ->shutdown
> callback is called ->shutdown because it is expected to shut the file
> system down.  That's why I suggested to Qu to add a new devloss callback,
> to describe that a device is lost.  In a file system with built-in
> redundancy that is not a shutdown.  So Qu, please add a devloss
> callback.  And maybe if we have no other good use for the shutdown
> callback we can remove it in favor of the devloss one.  But having
> something named shutdown take the block device and not always shutting
> the file system down is highly confusing.

I think we should add:

diff --git a/include/linux/fs.h b/include/linux/fs.h
index b085f161ed22..1d07f862a6a2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2368,6 +2368,7 @@ struct super_operations {
        long (*free_cached_objects)(struct super_block *,
                                    struct shrink_control *);
        void (*shutdown)(struct super_block *sb);
+       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
 };

You might want to drop a block device independent of whether the device
was somehow lost. So I find that a bit more flexible.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Jan Kara]

On Mon 23-06-25 15:04:51, Qu Wenruo wrote:
> 在 2025/6/23 14:45, Christoph Hellwig 写道:
> > On Fri, Jun 20, 2025 at 05:36:52PM +0200, Jan Kara wrote:
> > > On Fri 20-06-25 15:17:28, Qu Wenruo wrote:
> > > > Currently we already have the super_operations::shutdown() callback,
> > > > which is called when the block device of a filesystem is marked dead.
> > > > 
> > > > However this is mostly for single(ish) block device filesystems.
> > > > 
> > > > For multi-device filesystems, they may afford a missing device, thus may
> > > > continue work without fully shutdown the filesystem.
> > > > 
> > > > So add a new super_operation::shutdown_bdev() callback, for mutli-device
> > > > filesystems like btrfs and bcachefs.
> > > > 
> > > > For now the only user is fs_holder_ops::mark_dead(), which will call
> > > > shutdown_bdev() if supported.
> > > > If not supported then fallback to the original shutdown() callback.
> > > > 
> > > > Btrfs is going to add the usage of shutdown_bdev() soon.
> > > > 
> > > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > > 
> > > Thanks for the patch. I think that we could actually add 'bdev' that
> > > triggered shutdown among arguments ->shutdown takes instead of introducing
> > > a new handler.
> > 
> > I don't really think that's a good idea as-is.  The current ->shutdown
> > callback is called ->shutdown because it is expected to shut the file
> > system down.  That's why I suggested to Qu to add a new devloss callback,
> > to describe that a device is lost.  In a file system with built-in
> > redundancy that is not a shutdown.  So Qu, please add a devloss
> > callback.  And maybe if we have no other good use for the shutdown
> > callback we can remove it in favor of the devloss one.  But having
> > something named shutdown take the block device and not always shutting
> > the file system down is highly confusing.
> 
> OK, I got the point of the name "devloss" now, didn't notice the naming
> itself is important at that timing.
> 
> And in fact a new callback is much easier on me, no need to modify the code
> of other fses.
> 
> @Jan, would this be acceptable for a new devloss() callback instead?

OK, now I understand the rationale better as well. Yes, devloss() callback
makes sense to me. It just looked weird to have two types shutdown
callback. And yes, eventually we might provide a generic handler for
devloss callback that will just shut the filesystem down and then we can
remove the shutdown callback but that can happen later.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christoph Hellwig]

On Mon, Jun 23, 2025 at 12:56:28PM +0200, Christian Brauner wrote:
>         void (*shutdown)(struct super_block *sb);
> +       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
>  };
> 
> You might want to drop a block device independent of whether the device
> was somehow lost. So I find that a bit more flexible.

Drop is weird word for what is happening here, and if it wasn't for the
context in this thread I'd expect it to be about refcounting in Linux.

When the VFS/libfs does an upcall into the file system to notify it
that a device is gone that's pretty much a device loss.  I'm not married
to the exact name, but drop seems like a pretty bad choice.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/23 23:27, Christoph Hellwig 写道:
> On Mon, Jun 23, 2025 at 12:56:28PM +0200, Christian Brauner wrote:
>>          void (*shutdown)(struct super_block *sb);
>> +       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
>>   };
>>
>> You might want to drop a block device independent of whether the device
>> was somehow lost. So I find that a bit more flexible.
> 
> Drop is weird word for what is happening here, and if it wasn't for the
> context in this thread I'd expect it to be about refcounting in Linux.
> 
> When the VFS/libfs does an upcall into the file system to notify it
> that a device is gone that's pretty much a device loss.  I'm not married
> to the exact name, but drop seems like a pretty bad choice.

What about a more common used term, mark_dead()?

It's already used in blk_holder_ops, and I'd say it's more 
straighforward to me, compared to shutdown()/goingdown().

Thanks,
Qu

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christian Brauner]

On Tue, Jun 24, 2025 at 06:57:08AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/6/23 23:27, Christoph Hellwig 写道:
> > On Mon, Jun 23, 2025 at 12:56:28PM +0200, Christian Brauner wrote:
> > >          void (*shutdown)(struct super_block *sb);
> > > +       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> > >   };
> > > 
> > > You might want to drop a block device independent of whether the device
> > > was somehow lost. So I find that a bit more flexible.
> > 
> > Drop is weird word for what is happening here, and if it wasn't for the
> > context in this thread I'd expect it to be about refcounting in Linux.
> > 
> > When the VFS/libfs does an upcall into the file system to notify it
> > that a device is gone that's pretty much a device loss.  I'm not married
> > to the exact name, but drop seems like a pretty bad choice.
> 
> What about a more common used term, mark_dead()?
> 
> It's already used in blk_holder_ops, and I'd say it's more straighforward to
> me, compared to shutdown()/goingdown().

But it's not about the superblock going down necessarily. It's about the
device going away for whatever reason:

void (*yank_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
void (*pull_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
void (*unplug_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
void (*remove_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);

On a single device superblock unplugging that device would obviously
cause an actual shutdown. On multi-device superblocks it doesn't always.

(That brings me to another thought. Is there a use-case for knowing in
advance whether removing a device would shut down the superblock?
Because then the ability to probe whether a device can be safely
removed or an option to only remove the device if it can be removed
without killing the superblock would be a natural extension.)

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/24 18:21, Christian Brauner 写道:
> On Tue, Jun 24, 2025 at 06:57:08AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2025/6/23 23:27, Christoph Hellwig 写道:
>>> On Mon, Jun 23, 2025 at 12:56:28PM +0200, Christian Brauner wrote:
>>>>           void (*shutdown)(struct super_block *sb);
>>>> +       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
>>>>    };
>>>>
>>>> You might want to drop a block device independent of whether the device
>>>> was somehow lost. So I find that a bit more flexible.
>>>
>>> Drop is weird word for what is happening here, and if it wasn't for the
>>> context in this thread I'd expect it to be about refcounting in Linux.
>>>
>>> When the VFS/libfs does an upcall into the file system to notify it
>>> that a device is gone that's pretty much a device loss.  I'm not married
>>> to the exact name, but drop seems like a pretty bad choice.
>>
>> What about a more common used term, mark_dead()?
>>
>> It's already used in blk_holder_ops, and I'd say it's more straighforward to
>> me, compared to shutdown()/goingdown().
> 
> But it's not about the superblock going down necessarily. It's about the
> device going away for whatever reason:
> 
> void (*yank_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*pull_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*unplug_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*remove_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);

All sound good to me, although the last one sounds better.

> 
> On a single device superblock unplugging that device would obviously
> cause an actual shutdown. On multi-device superblocks it doesn't always.
> 
> (That brings me to another thought. Is there a use-case for knowing in
> advance whether removing a device would shut down the superblock?

Maybe another interface like can_remove_bdev()?

It's not hard for btrfs to provide it, we already have a check function 
btrfs_check_rw_degradable() to do that.

Although I'd say, that will be something way down the road.

We even don't have a proper way to let end user configure the device 
loss behavior.
E.g. some end users may prefer a full shutdown to be extra cautious, 
other than continue degraded.

Thanks,
Qu

> Because then the ability to probe whether a device can be safely
> removed or an option to only remove the device if it can be removed
> without killing the superblock would be a natural extension.)

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christian Brauner]

On Tue, Jun 24, 2025 at 06:36:01PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/6/24 18:21, Christian Brauner 写道:
> > On Tue, Jun 24, 2025 at 06:57:08AM +0930, Qu Wenruo wrote:
> > > 
> > > 
> > > 在 2025/6/23 23:27, Christoph Hellwig 写道:
> > > > On Mon, Jun 23, 2025 at 12:56:28PM +0200, Christian Brauner wrote:
> > > > >           void (*shutdown)(struct super_block *sb);
> > > > > +       void (*drop_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> > > > >    };
> > > > > 
> > > > > You might want to drop a block device independent of whether the device
> > > > > was somehow lost. So I find that a bit more flexible.
> > > > 
> > > > Drop is weird word for what is happening here, and if it wasn't for the
> > > > context in this thread I'd expect it to be about refcounting in Linux.
> > > > 
> > > > When the VFS/libfs does an upcall into the file system to notify it
> > > > that a device is gone that's pretty much a device loss.  I'm not married
> > > > to the exact name, but drop seems like a pretty bad choice.
> > > 
> > > What about a more common used term, mark_dead()?
> > > 
> > > It's already used in blk_holder_ops, and I'd say it's more straighforward to
> > > me, compared to shutdown()/goingdown().
> > 
> > But it's not about the superblock going down necessarily. It's about the
> > device going away for whatever reason:
> > 
> > void (*yank_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> > void (*pull_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> > void (*unplug_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> > void (*remove_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> 
> All sound good to me, although the last one sounds better.
> 
> > 
> > On a single device superblock unplugging that device would obviously
> > cause an actual shutdown. On multi-device superblocks it doesn't always.
> > 
> > (That brings me to another thought. Is there a use-case for knowing in
> > advance whether removing a device would shut down the superblock?
> 
> Maybe another interface like can_remove_bdev()?
> 
> It's not hard for btrfs to provide it, we already have a check function
> btrfs_check_rw_degradable() to do that.
> 
> Although I'd say, that will be something way down the road.

Yes, for sure. I think long-term we should hoist at least the bare
infrastructure for multi-device filesystem management into the VFS.
Or we should at least explore whether that's feasible and if it's
overall advantageous to maintenance and standardization. We've already
done a bit of that and imho it's now a lot easier to reason about the
basics already.

> 
> We even don't have a proper way to let end user configure the device loss
> behavior.
> E.g. some end users may prefer a full shutdown to be extra cautious, other
> than continue degraded.

Right.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/24 18:43, Christian Brauner 写道:
[...]
>> It's not hard for btrfs to provide it, we already have a check function
>> btrfs_check_rw_degradable() to do that.
>>
>> Although I'd say, that will be something way down the road.
> 
> Yes, for sure. I think long-term we should hoist at least the bare
> infrastructure for multi-device filesystem management into the VFS.

Just want to mention that, "multi-device filesystem" already includes 
fses with external journal.

Thus the new callback may be a good chance for those mature fses to 
explore some corner case availability improvement, e.g. the loss of the 
external journal device while there is no live journal on it.
(I have to admin it's super niche, and live-migration to internal 
journal may be way more complex than my uneducated guess)

Thanks,
Qu

> Or we should at least explore whether that's feasible and if it's
> overall advantageous to maintenance and standardization. We've already
> done a bit of that and imho it's now a lot easier to reason about the
> basics already.
> 
>>
>> We even don't have a proper way to let end user configure the device loss
>> behavior.
>> E.g. some end users may prefer a full shutdown to be extra cautious, other
>> than continue degraded.
> 
> Right.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christian Brauner]

On Tue, Jun 24, 2025 at 07:21:50PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/6/24 18:43, Christian Brauner 写道:
> [...]
> > > It's not hard for btrfs to provide it, we already have a check function
> > > btrfs_check_rw_degradable() to do that.
> > > 
> > > Although I'd say, that will be something way down the road.
> > 
> > Yes, for sure. I think long-term we should hoist at least the bare
> > infrastructure for multi-device filesystem management into the VFS.
> 
> Just want to mention that, "multi-device filesystem" already includes fses
> with external journal.

Yes, that's what I meant below by "We've already done a bit of that".
It's now possible to actually reach all devices associted with a
filesystem from the block layer. It works for xfs and ext4 with
journal fileystems. So for example, you can freeze the log device and
the main device as the block layer is now able to find both and the fs
stays frozen until both have been unfrozen. This wasn't possible before
the rework we did.

Now follows a tiny rant not targeted at you specifically but something
that still bugs me in general:

We had worked on extending this to btrfs so that it's all integrated
properly with the block layer. And we heard long promises of how you
would make that switch happen but refused us to let us make that switch.
So now it's 2 years later and zero happend in that area.

That also means block device freezing on btrfs is broken. If you freeze
a block device used by btrfs via the dm (even though unlikely) layer you
freeze the block device without btrfs being informed about that.

It also means that block device removal is likely a bit yanky because
btrfs won't be notified when any device other than the main device is
suddenly yanked. You probably have logic there but the block layer can
easily inform the filesystem about such an event nowadays and let it
take appropriate action.

And fwiw, you also don't restrict writing to mounted block devices.
That's another thing you blocked us from implementing even though we
sent the changes for that already and so we disabled that in
ead622674df5 ("btrfs: Do not restrict writes to btrfs devices"). So
you're also still vulnerable to that stuff.

> 
> Thus the new callback may be a good chance for those mature fses to explore
> some corner case availability improvement, e.g. the loss of the external
> journal device while there is no live journal on it.

Already handled for xfs and ext4 cleanly since our rework iiuc.

> (I have to admin it's super niche, and live-migration to internal journal
> may be way more complex than my uneducated guess)
> 
> Thanks,
> Qu
> 
> > Or we should at least explore whether that's feasible and if it's
> > overall advantageous to maintenance and standardization. We've already
> > done a bit of that and imho it's now a lot easier to reason about the
> > basics already.
> > 
> > > 
> > > We even don't have a proper way to let end user configure the device loss
> > > behavior.
> > > E.g. some end users may prefer a full shutdown to be extra cautious, other
> > > than continue degraded.
> > 
> > Right.
> 

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christoph Hellwig]

On Tue, Jun 24, 2025 at 06:57:08AM +0930, Qu Wenruo wrote:
> > When the VFS/libfs does an upcall into the file system to notify it
> > that a device is gone that's pretty much a device loss.  I'm not married
> > to the exact name, but drop seems like a pretty bad choice.
> 
> What about a more common used term, mark_dead()?

That's even less generic than devloss.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christoph Hellwig]

On Tue, Jun 24, 2025 at 10:51:51AM +0200, Christian Brauner wrote:
> 
> void (*yank_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*pull_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*unplug_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);
> void (*remove_bdev)(struct super_block *sb, struct block_device *bdev /* , unsigned int flags/reason maybe too ? */);

Out of these remove_bdev is the most sane.

> (That brings me to another thought. Is there a use-case for knowing in
> advance whether removing a device would shut down the superblock?
> Because then the ability to probe whether a device can be safely
> removed or an option to only remove the device if it can be removed
> without killing the superblock would be a natural extension.)

I don't think there is a use for it at this (holder_ops) level.  If
the device driver notifies about a device going to away we're already
committed.  It makes some sense at the file system administration level,
but you want to tie that much deepter into the file system, i.e. you'd
actually usually want to migrate all data off the device first.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Christoph Hellwig]

On Tue, Jun 24, 2025 at 07:21:50PM +0930, Qu Wenruo wrote:
> Thus the new callback may be a good chance for those mature fses to explore
> some corner case availability improvement, e.g. the loss of the external
> journal device while there is no live journal on it.
> (I have to admin it's super niche, and live-migration to internal journal
> may be way more complex than my uneducated guess)

I don't know know of a file system that has a log/journal where is isn't
integral part of the metadata write architecture.

Re: [PATCH RFC 5/6] fs: introduce a shutdown_bdev super block operation

[Qu Wenruo]

在 2025/6/24 19:45, Christian Brauner 写道:
> On Tue, Jun 24, 2025 at 07:21:50PM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2025/6/24 18:43, Christian Brauner 写道:
>> [...]
>>>> It's not hard for btrfs to provide it, we already have a check function
>>>> btrfs_check_rw_degradable() to do that.
>>>>
>>>> Although I'd say, that will be something way down the road.
>>>
>>> Yes, for sure. I think long-term we should hoist at least the bare
>>> infrastructure for multi-device filesystem management into the VFS.
>>
>> Just want to mention that, "multi-device filesystem" already includes fses
>> with external journal.
> 
> Yes, that's what I meant below by "We've already done a bit of that".
> It's now possible to actually reach all devices associted with a
> filesystem from the block layer. It works for xfs and ext4 with
> journal fileystems. So for example, you can freeze the log device and
> the main device as the block layer is now able to find both and the fs
> stays frozen until both have been unfrozen. This wasn't possible before
> the rework we did.
> 
> Now follows a tiny rant not targeted at you specifically but something
> that still bugs me in general:
> 
> We had worked on extending this to btrfs so that it's all integrated
> properly with the block layer. And we heard long promises of how you
> would make that switch happen but refused us to let us make that switch.
> So now it's 2 years later and zero happend in that area.

I believe we (the btrfs community) are not intentionally rejecting this, 
bad luck and lack of review bandwidth is involved, as at that time we're 
focusing on migrating to the new fsconfig mount interface.

That delayed review of the original patchset from Christoph, then the 
old series no longer applies due to the fsconfig change, until Johannes 
revived the series for the first time.

Then even more (minor) conflicts with recent mount ro/rw mount fixes, 
and although Johannes tried his best to refresh the series, those 
conflicts eventually resulted test failures.


And I wasn't even following all those updates, until one day I'm 
eventually freed from btrfs large folio support, and had time to attack 
the long failure generic/730 due to lack of shutdown support.

Then I was dragged into the rabbit hole and finally we're here.


Also I have to admit, at least me do not have much experience in the 
block/VFS field, and sometimes we still assume the existing 
infrastructure is still mostly targeting single-ish block device 
filesystems, but it's not true anymore.

We're improving this, and got quite some help from Christoph, e.g. he 
contributed the btrfs bio layer to do all the bio split/chain inside btrfs.

I hope this remove_bdev() call back can be a good start point to bridge 
the btrfs and block community closer.

> 
> That also means block device freezing on btrfs is broken. If you freeze
> a block device used by btrfs via the dm (even though unlikely) layer you
> freeze the block device without btrfs being informed about that.

Yes, you're totally right and I also believe that may be the reason of 
btrfs corruption after hibernation/suspension.

> 
> It also means that block device removal is likely a bit yanky because
> btrfs won't be notified when any device other than the main device is
> suddenly yanked. You probably have logic there but the block layer can
> easily inform the filesystem about such an event nowadays and let it
> take appropriate action.

Yep, btrfs doesn't handling removal of devices at runtime at all, but 
still tries to do IO on that device, only saved by the extra mirrors.

Meaning unless a user is monitoring the dmesg, one won't notice the 
problem, which a huge degradation of availability happening silently.

> 
> And fwiw, you also don't restrict writing to mounted block devices.
> That's another thing you blocked us from implementing even though we
> sent the changes for that already and so we disabled that in
> ead622674df5 ("btrfs: Do not restrict writes to btrfs devices"). So
> you're also still vulnerable to that stuff.

Oh, that's something new and let me explore this after all the 
remove_bdev() callback thing.

Thanks,
Qu

> 
>>
>> Thus the new callback may be a good chance for those mature fses to explore
>> some corner case availability improvement, e.g. the loss of the external
>> journal device while there is no live journal on it.
> 
> Already handled for xfs and ext4 cleanly since our rework iiuc.
> 
>> (I have to admin it's super niche, and live-migration to internal journal
>> may be way more complex than my uneducated guess)
>>
>> Thanks,
>> Qu
>>
>>> Or we should at least explore whether that's feasible and if it's
>>> overall advantageous to maintenance and standardization. We've already
>>> done a bit of that and imho it's now a lot easier to reason about the
>>> basics already.
>>>
>>>>
>>>> We even don't have a proper way to let end user configure the device loss
>>>> behavior.
>>>> E.g. some end users may prefer a full shutdown to be extra cautious, other
>>>> than continue degraded.
>>>
>>> Right.
>>
>