linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] btrfs: add remove_bdev() callback
@ 2025-07-01  5:32 Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

[CHANGELOG]
v2:
- Enhance and rename shutdown() callback
  Rename it to remove_bdev() and add a @bdev parameter.
  For the existing call backs in filesystems, keep their callback
  function names, now something like ".remove_bdev = ext4_shutdown,"
  will be a quick indicator of the behavior.

- Remove the @surprise parameter for the remove_bdev() parameter.
  The fs_bdev_mark_dead() is already trying to sync the fs if it's not
  a surprise removal.
  So there isn't much a filesystem can do with the @surprise parameter.

- Fix btrfs error handling when the devices are not opened
  There are several cases that the fs_devices is not opened, including:
  * sget_fc() failure
  * an existing super block is returned
  * a new super block is returned but btrfS_open_fs_devices() failed

  Handle the error properly so that fs_devices is not freed twice.

RFC->v1:
- Add a new remove_bdev() callback
  Thanks all the feedback from Christian, Christoph and Jan on this new
  name.

- Add a @surprise parameter to the remove_bdev() callback
  To keep it the same as the bdev_mark_dead().

- Hide the shutdown ioctl and remove_bdev callback behind experimental 
  With the shutdown ioctl, there are at least 2 test failures (g/388, g/508).

  G/388 is related to the error handling with COW fixup.
  G/508 looks like something related to log replay.

  And the remove_bdev() doesn't have any btrfs specific test case yet to
  check the auto-degraded behavior, nor the auto-degraded behavior is
  fully discussed.

  So hide both of them behind experimental features.

- Do not use btrfs_handle_fs_error() to avoid freeze/thaw behavior change
  btrfs_handle_fs_error() will flips the fs read-only, which will
  affect freeze/thaw behavior.
  And no other fs set the fs read-only when shutting down, so follow the
  other fs to have a more consistent behavior.


Qu Wenruo (6):
  fs: enhance and rename shutdown() callback to remove_bdev()
  btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN
  btrfs: reject file operations if in shutdown state
  btrfs: reject delalloc ranges if in shutdown state
  btrfs: implement shutdown ioctl
  btrfs: implement remove_bdev super operation callback

 fs/btrfs/file.c            | 25 ++++++++++++++++++++-
 fs/btrfs/fs.h              | 28 ++++++++++++++++++++++++
 fs/btrfs/inode.c           | 14 +++++++++++-
 fs/btrfs/ioctl.c           | 43 ++++++++++++++++++++++++++++++++++++
 fs/btrfs/messages.c        |  1 +
 fs/btrfs/reflink.c         |  3 +++
 fs/btrfs/super.c           | 45 ++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.c         |  2 ++
 fs/btrfs/volumes.h         |  5 +++++
 fs/exfat/super.c           |  4 ++--
 fs/ext4/super.c            |  4 ++--
 fs/f2fs/super.c            |  4 ++--
 fs/ntfs3/super.c           |  4 ++--
 fs/super.c                 |  4 ++--
 fs/xfs/xfs_super.c         |  5 +++--
 include/linux/fs.h         |  7 +++++-
 include/uapi/linux/btrfs.h |  9 ++++++++
 17 files changed, 192 insertions(+), 15 deletions(-)

-- 
2.50.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-01  5:54   ` Darrick J. Wong
                     ` (2 more replies)
  2025-07-01  5:32 ` [PATCH v2 2/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN Qu Wenruo
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel
  Cc: viro, brauner, jack, linux-ext4, linux-f2fs-devel, ntfs3,
	linux-xfs

Currently all the filesystems implementing the
super_opearations::shutdown() call back can not afford losing a device.

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:

- Rename shutdown() call back to remove_bdev()
  To better describe when the call back 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.
Since the behavior is still to shutdown the fs, they shouldn't change
their function names.

This has a good side effect that, a single line like
".remove_bdev = ext4_shutdown," will easily show the fs behavior and
indicate the fs will shutdown when a device went missing.

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>
---
 fs/exfat/super.c   | 4 ++--
 fs/ext4/super.c    | 4 ++--
 fs/f2fs/super.c    | 4 ++--
 fs/ntfs3/super.c   | 4 ++--
 fs/super.c         | 4 ++--
 fs/xfs/xfs_super.c | 5 +++--
 include/linux/fs.h | 7 ++++++-
 7 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 7ed858937d45..5773026be84c 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_shutdown(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_shutdown,
 };
 
 enum {
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c7d39da7e733..8724f89d20d8 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_shutdown(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_shutdown,
 #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..51c60b429a31 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_shutdown(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_shutdown,
 };
 
 #ifdef CONFIG_FS_ENCRYPTION
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 920a1ab47b63..5e422543b851 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -764,7 +764,7 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 /*
  * ntfs_shutdown - super_operations::shutdown
  */
-static void ntfs_shutdown(struct super_block *sb)
+static void ntfs_shutdown(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_shutdown,
 	.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..e47d427f4416 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1277,7 +1277,8 @@ xfs_fs_free_cached_objects(
 
 static void
 xfs_fs_shutdown(
-	struct super_block	*sb)
+	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_shutdown,
 	.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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 3/6] btrfs: reject file operations if in shutdown state Qu Wenruo
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

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

A new helper, btrfs_force_shutdown() is introduced, which will:

- Mark the fs as error
  But without flipping the fs read-only.
  This is a special handling for the future shutdown ioctl, which will
  freeze the fs first, set the SHUTDOWN flag, thaw the fs.

  But the thaw path will no longer call the unfreeze_fs() call back
  if the superblock is already read-only.

  So to handle future shutdown correctly, we only mark the fs as error,
  without flipping it read-only.

- Set the SHUTDOWN flag and output an message

New users of those interfaces will be added when implementing shutdown
ioctl support.

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

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index d90304d4e32c..45185c095696 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,27 @@ 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_force_shutdown(struct btrfs_fs_info *fs_info)
+{
+	/*
+	 * Here we do not want to use handle_fs_error(), which will mark
+	 * the fs read-only.
+	 * Some call sites like shutdown ioctl will mark the fs shutdown
+	 * when the fs is frozen. But thaw path will handle RO and RW fs
+	 * differently.
+	 *
+	 * So here we only mark the fs error without flipping it RO.
+	 */
+	WRITE_ONCE(fs_info->fs_error, -EIO);
+	if (!test_and_set_bit(BTRFS_FS_STATE_EMERGENCY_SHUTDOWN, &fs_info->fs_state))
+		btrfs_info(fs_info, "emergency shutdown");
+}
+
 /*
  * 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.50.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/6] btrfs: reject file operations if in shutdown state
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 2/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 4/6] btrfs: reject delalloc ranges " Qu Wenruo
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

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 247676f34f2e..918c39a5ed39 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1419,6 +1419,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
@@ -1981,6 +1983,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;
 
@@ -3040,6 +3044,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;
@@ -3731,6 +3738,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);
@@ -3743,6 +3753,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) ||
@@ -3753,10 +3766,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 65763bc6a0f6..d86967bd3c9c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5066,6 +5066,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.50.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/6] btrfs: reject delalloc ranges if in shutdown state
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
                   ` (2 preceding siblings ...)
  2025-07-01  5:32 ` [PATCH v2 3/6] btrfs: reject file operations if in shutdown state Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 5/6] btrfs: implement shutdown ioctl Qu Wenruo
  2025-07-01  5:32 ` [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback Qu Wenruo
  5 siblings, 0 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

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 80c72c594b19..bb2b5d594b14 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.50.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/6] btrfs: implement shutdown ioctl
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
                   ` (3 preceding siblings ...)
  2025-07-01  5:32 ` [PATCH v2 4/6] btrfs: reject delalloc ranges " Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-02  9:38   ` kernel test robot
  2025-07-01  5:32 ` [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback Qu Wenruo
  5 siblings, 1 reply; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

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

For now btrfs don't distinguish DEFAULT and LOGFLUSH flags (just like
f2fs), both will freeze the fs first (implies committing the current
transaction), setting the SHUTDOWN flag and finally thaw the fs.

For NOLOGFLUSH flag, the freeze/thaw part is skipped thus the current
transaction is aborted.

The new shutdown ioctl is hidden behind experimental features for more
testing.

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

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d86967bd3c9c..05b13d66f0f7 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5212,6 +5212,36 @@ static int btrfs_ioctl_subvol_sync(struct btrfs_fs_info *fs_info, void __user *a
 	return 0;
 }
 
+#ifdef CONFIG_BTRFS_EXPERIMENTAL
+static int btrfs_emergency_shutdown(struct btrfs_fs_info *fs_info, u32 flags)
+{
+	int ret = 0;
+
+	if (flags >= BTRFS_SHUTDOWN_FLAGS_LAST)
+		return -EINVAL;
+
+	if (btrfs_is_shutdown(fs_info))
+		return 0;
+
+	switch (flags) {
+	case BTRFS_SHUTDOWN_FLAGS_LOGFLUSH:
+	case BTRFS_SHUTDOWN_FLAGS_DEFAULT:
+		ret = freeze_super(fs_info->sb, FREEZE_HOLDER_KERNEL, NULL);
+		if (ret)
+			return ret;
+		btrfs_force_shutdown(fs_info);
+		ret = thaw_super(fs_info->sb, FREEZE_HOLDER_KERNEL, NULL);
+		if (ret)
+			return ret;
+		break;
+	case BTRFS_SHUTDOWN_FLAGS_NOLOGFLUSH:
+		btrfs_force_shutdown(fs_info);
+		break;
+	}
+	return ret;
+}
+#endif
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -5219,6 +5249,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 	struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	void __user *argp = (void __user *)arg;
+	/* If @arg is just an unsigned long value. */
+	unsigned long flags;
 
 	switch (cmd) {
 	case FS_IOC_GETVERSION:
@@ -5367,6 +5399,14 @@ long btrfs_ioctl(struct file *file, unsigned int
 #endif
 	case BTRFS_IOC_SUBVOL_SYNC_WAIT:
 		return btrfs_ioctl_subvol_sync(fs_info, argp);
+#ifdef CONFIG_BTRFS_EXPERIMENTAL
+	case BTRFS_IOC_SHUTDOWN:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (get_user(flags, (__u32 __user *)arg))
+			return -EFAULT;
+		return btrfs_emergency_shutdown(fs_info, flags);
+#endif
 	}
 
 	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.50.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback
  2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
                   ` (4 preceding siblings ...)
  2025-07-01  5:32 ` [PATCH v2 5/6] btrfs: implement shutdown ioctl Qu Wenruo
@ 2025-07-01  5:32 ` Qu Wenruo
  2025-07-01  8:21   ` Anand Jain
  5 siblings, 1 reply; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  5:32 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

For this callback, btrfs will:

- Go degraded if the fs can still maintain RW operations
  And of course mark the target device as missing.

- Shutdown if the fs can not maintain RW operations

To support the lookup from bdev to a btrfs_device,
btrfs_dev_lookup_args is enhanced to have a new @devt member.
If set, we should be able to use that @devt member to uniquely locating a
btrfs device.

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%
chance to got some good data.

But a filesystem returning -EIO for half of the time is not really
considered usable.
Further 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.

And the remove_bdev callback will be hidden behind experimental features
for now, the reasons are:

- There are not enough btrfs specific bdev removal test cases
  The existing test cases are all removing the only device, thus only
  exercises the shutdown branch.

- Not yet determined what's the expected behavior
  Although the current auto-degrade behavior is no worse than the old
  behavior, it may not always be what the end users want.

  Before there is a concrete interface, better hide the new feature
  from end users.

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

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 727d5c1d1bf1..d3ee4740fd22 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2421,6 +2421,48 @@ static long btrfs_free_cached_objects(struct super_block *sb, struct shrink_cont
 	return 0;
 }
 
+#ifdef CONFIG_BTRFS_EXPERIMENTAL
+static void btrfs_remove_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 };
+	bool can_rw;
+
+	mutex_lock(&fs_info->fs_devices->device_list_mutex);
+	device = btrfs_find_device(fs_info->fs_devices, &lookup_args);
+	if (!device) {
+		btrfs_warn(fs_info, "unable to find btrfs device for block device '%pg'",
+			   bdev);
+		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+		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--;
+	}
+	can_rw = btrfs_check_rw_degradable(fs_info, device);
+	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+	/*
+	 * Now device is considered missing, btrfs_device_name() won't give a
+	 * meaningful result anymore, so only output the devid.
+	 */
+	if (!can_rw) {
+		btrfs_crit(fs_info,
+		"btrfs device id %llu has gone missing, can not maintain read-write",
+			   device->devid);
+		btrfs_force_shutdown(fs_info);
+		return;
+	}
+	btrfs_warn(fs_info,
+		   "btrfs device id %llu has gone missing, continue as degraded",
+		   device->devid);
+	btrfs_set_opt(fs_info->mount_opt, DEGRADED);
+}
+#endif
+
 static const struct super_operations btrfs_super_ops = {
 	.drop_inode	= btrfs_drop_inode,
 	.evict_inode	= btrfs_evict_inode,
@@ -2436,6 +2478,9 @@ 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,
+#ifdef CONFIG_BTRFS_EXPERIMENTAL
+	.remove_bdev	= btrfs_remove_bdev,
+#endif
 };
 
 static const struct file_operations btrfs_ctl_fops = {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8ea1a69808a3..8feac0129bdd 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6794,6 +6794,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 6acb154ccf87..71e570f8337d 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -663,6 +663,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.50.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
@ 2025-07-01  5:54   ` Darrick J. Wong
  2025-07-01  6:14   ` Christoph Hellwig
  2025-07-01  7:52   ` Jan Kara
  2 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2025-07-01  5:54 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: linux-btrfs, linux-fsdevel, viro, brauner, jack, linux-ext4,
	linux-f2fs-devel, ntfs3, linux-xfs

On Tue, Jul 01, 2025 at 03:02:34PM +0930, Qu Wenruo wrote:
> Currently all the filesystems implementing the
> super_opearations::shutdown() call back can not afford losing a device.
> 
> 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:
> 
> - Rename shutdown() call back to remove_bdev()
>   To better describe when the call back 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.
> Since the behavior is still to shutdown the fs, they shouldn't change
> their function names.
> 
> This has a good side effect that, a single line like
> ".remove_bdev = ext4_shutdown," will easily show the fs behavior and
> indicate the fs will shutdown when a device went missing.
> 
> Btrfs is going to implement the callback soon, which will either
> shutdown the fs or continue read-write operations.

Hrmm, this could be useful for xfs rt devices, if we could some day
reattach a resurrected bdev to a still-running filesystem....

Looks good to me,
Acked-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> 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>
> ---
>  fs/exfat/super.c   | 4 ++--
>  fs/ext4/super.c    | 4 ++--
>  fs/f2fs/super.c    | 4 ++--
>  fs/ntfs3/super.c   | 4 ++--
>  fs/super.c         | 4 ++--
>  fs/xfs/xfs_super.c | 5 +++--
>  include/linux/fs.h | 7 ++++++-
>  7 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 7ed858937d45..5773026be84c 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_shutdown(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_shutdown,
>  };
>  
>  enum {
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c7d39da7e733..8724f89d20d8 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_shutdown(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_shutdown,
>  #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..51c60b429a31 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_shutdown(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_shutdown,
>  };
>  
>  #ifdef CONFIG_FS_ENCRYPTION
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 920a1ab47b63..5e422543b851 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -764,7 +764,7 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
>  /*
>   * ntfs_shutdown - super_operations::shutdown
>   */
> -static void ntfs_shutdown(struct super_block *sb)
> +static void ntfs_shutdown(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_shutdown,
>  	.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..e47d427f4416 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1277,7 +1277,8 @@ xfs_fs_free_cached_objects(
>  
>  static void
>  xfs_fs_shutdown(
> -	struct super_block	*sb)
> +	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_shutdown,
>  	.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
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
  2025-07-01  5:54   ` Darrick J. Wong
@ 2025-07-01  6:14   ` Christoph Hellwig
  2025-07-01  6:35     ` Qu Wenruo
  2025-07-01  7:52   ` Jan Kara
  2 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2025-07-01  6:14 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: linux-btrfs, linux-fsdevel, viro, brauner, jack, linux-ext4,
	linux-f2fs-devel, ntfs3, linux-xfs

On Tue, Jul 01, 2025 at 03:02:34PM +0930, Qu Wenruo wrote:
> To allow those multi-device filesystems to be integrated to use
> fs_holder_ops:
> 
> - Rename shutdown() call back to remove_bdev()
>   To better describe when the call back is called.

What is renamed back here?

> -static void exfat_shutdown(struct super_block *sb)
> +static void exfat_shutdown(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_shutdown,

Please also rename the function so that they match the method name.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  6:14   ` Christoph Hellwig
@ 2025-07-01  6:35     ` Qu Wenruo
  2025-07-01  6:40       ` Christoph Hellwig
  2025-07-01  8:41       ` Christian Brauner
  0 siblings, 2 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  6:35 UTC (permalink / raw)
  To: Christoph Hellwig, Qu Wenruo
  Cc: linux-btrfs, linux-fsdevel, viro, brauner, jack, linux-ext4,
	linux-f2fs-devel, ntfs3, linux-xfs



在 2025/7/1 15:44, Christoph Hellwig 写道:
> On Tue, Jul 01, 2025 at 03:02:34PM +0930, Qu Wenruo wrote:
>> To allow those multi-device filesystems to be integrated to use
>> fs_holder_ops:
>>
>> - Rename shutdown() call back to remove_bdev()
>>    To better describe when the call back is called.
> 
> What is renamed back here?

Rename the old shutdown to remove_bdev().

> 
>> -static void exfat_shutdown(struct super_block *sb)
>> +static void exfat_shutdown(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_shutdown,
> 
> Please also rename the function so that they match the method name.

I prefer not, and it is intentionally left as is.

This give us a very clear view what a fs is expected to do.

If a fs can only shutdown when losing any device, a read won't need to 
dig into the details, just looking at that line will tell us what is the 
behavior.

Thanks,
Qu


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  6:35     ` Qu Wenruo
@ 2025-07-01  6:40       ` Christoph Hellwig
  2025-07-01  8:41       ` Christian Brauner
  1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2025-07-01  6:40 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Christoph Hellwig, Qu Wenruo, linux-btrfs, linux-fsdevel, viro,
	brauner, jack, linux-ext4, linux-f2fs-devel, ntfs3, linux-xfs

On Tue, Jul 01, 2025 at 04:05:03PM +0930, Qu Wenruo wrote:
> > Please also rename the function so that they match the method name.
> 
> I prefer not, and it is intentionally left as is.
> 
> This give us a very clear view what a fs is expected to do.

No.  Mismatching method are an insane pest that makes refactoring
or even just reading the code painful as hell.

Nacked-by: Christoph Hellwig <hch@lst.de>

without that.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
  2025-07-01  5:54   ` Darrick J. Wong
  2025-07-01  6:14   ` Christoph Hellwig
@ 2025-07-01  7:52   ` Jan Kara
  2 siblings, 0 replies; 17+ messages in thread
From: Jan Kara @ 2025-07-01  7:52 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: linux-btrfs, linux-fsdevel, viro, brauner, jack, linux-ext4,
	linux-f2fs-devel, ntfs3, linux-xfs

On Tue 01-07-25 15:02:34, Qu Wenruo wrote:
> Currently all the filesystems implementing the
> super_opearations::shutdown() call back can not afford losing a device.
> 
> 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:
> 
> - Rename shutdown() call back to remove_bdev()
>   To better describe when the call back 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.
> Since the behavior is still to shutdown the fs, they shouldn't change
> their function names.
> 
> This has a good side effect that, a single line like
> ".remove_bdev = ext4_shutdown," will easily show the fs behavior and
> indicate the fs will shutdown when a device went missing.
> 
> 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>

So I don't feel as strongly as Christoph about individual filesystem method
names but I tend to agree that over long term and large codebase
mismatching hook & implementation names does more harm than good (it just
creates these WTH is this moments couple years down the road when you
already forget about the original motivation). So I'd prefer they are named
like ext4_remove_bdev() etc. Otherwise the patch looks good!

								Honza

> ---
>  fs/exfat/super.c   | 4 ++--
>  fs/ext4/super.c    | 4 ++--
>  fs/f2fs/super.c    | 4 ++--
>  fs/ntfs3/super.c   | 4 ++--
>  fs/super.c         | 4 ++--
>  fs/xfs/xfs_super.c | 5 +++--
>  include/linux/fs.h | 7 ++++++-
>  7 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 7ed858937d45..5773026be84c 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_shutdown(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_shutdown,
>  };
>  
>  enum {
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c7d39da7e733..8724f89d20d8 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_shutdown(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_shutdown,
>  #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..51c60b429a31 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_shutdown(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_shutdown,
>  };
>  
>  #ifdef CONFIG_FS_ENCRYPTION
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 920a1ab47b63..5e422543b851 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -764,7 +764,7 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
>  /*
>   * ntfs_shutdown - super_operations::shutdown
>   */
> -static void ntfs_shutdown(struct super_block *sb)
> +static void ntfs_shutdown(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_shutdown,
>  	.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..e47d427f4416 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1277,7 +1277,8 @@ xfs_fs_free_cached_objects(
>  
>  static void
>  xfs_fs_shutdown(
> -	struct super_block	*sb)
> +	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_shutdown,
>  	.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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback
  2025-07-01  5:32 ` [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback Qu Wenruo
@ 2025-07-01  8:21   ` Anand Jain
  2025-07-01  8:30     ` Qu Wenruo
  0 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2025-07-01  8:21 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack



> +#ifdef CONFIG_BTRFS_EXPERIMENTAL
> +static void btrfs_remove_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 };
> +	bool can_rw;
> +
> +	mutex_lock(&fs_info->fs_devices->device_list_mutex);
> +	device = btrfs_find_device(fs_info->fs_devices, &lookup_args);
> +	if (!device) {
> +		btrfs_warn(fs_info, "unable to find btrfs device for block device '%pg'",
> +			   bdev);
> +		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
> +		return;
> +	}
> +	set_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
> +	device->fs_devices->missing_devices++;

Where do we ensure that the block device wasn't already marked as
missing? If there's no such check, could missing_devices end up
exceeding total_devices?

Thanks, Anand

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback
  2025-07-01  8:21   ` Anand Jain
@ 2025-07-01  8:30     ` Qu Wenruo
  0 siblings, 0 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  8:30 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack



在 2025/7/1 17:51, Anand Jain 写道:
> 
> 
>> +#ifdef CONFIG_BTRFS_EXPERIMENTAL
>> +static void btrfs_remove_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 };
>> +    bool can_rw;
>> +
>> +    mutex_lock(&fs_info->fs_devices->device_list_mutex);
>> +    device = btrfs_find_device(fs_info->fs_devices, &lookup_args);
>> +    if (!device) {
>> +        btrfs_warn(fs_info, "unable to find btrfs device for block 
>> device '%pg'",
>> +               bdev);
>> +        mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>> +        return;
>> +    }
>> +    set_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
>> +    device->fs_devices->missing_devices++;
> 
> Where do we ensure that the block device wasn't already marked as
> missing? If there's no such check, could missing_devices end up
> exceeding total_devices?

Right, I'll change the device number related changes behind a 
test_and_set_bit(), so that we won't double accounting the missing device.

Thanks,
Qu>
> Thanks, Anand


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  6:35     ` Qu Wenruo
  2025-07-01  6:40       ` Christoph Hellwig
@ 2025-07-01  8:41       ` Christian Brauner
  2025-07-01  8:46         ` Qu Wenruo
  1 sibling, 1 reply; 17+ messages in thread
From: Christian Brauner @ 2025-07-01  8:41 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Christoph Hellwig, Qu Wenruo, linux-btrfs, linux-fsdevel, viro,
	jack, linux-ext4, linux-f2fs-devel, ntfs3, linux-xfs

On Tue, Jul 01, 2025 at 04:05:03PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/7/1 15:44, Christoph Hellwig 写道:
> > On Tue, Jul 01, 2025 at 03:02:34PM +0930, Qu Wenruo wrote:
> > > To allow those multi-device filesystems to be integrated to use
> > > fs_holder_ops:
> > > 
> > > - Rename shutdown() call back to remove_bdev()
> > >    To better describe when the call back is called.
> > 
> > What is renamed back here?
> 
> Rename the old shutdown to remove_bdev().
> 
> > 
> > > -static void exfat_shutdown(struct super_block *sb)
> > > +static void exfat_shutdown(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_shutdown,
> > 
> > Please also rename the function so that they match the method name.
> 
> I prefer not, and it is intentionally left as is.
> 
> This give us a very clear view what a fs is expected to do.

Qu, would you please rename the individual functions?

The NAK later just because of this is unnecessary. I will say clearly
that I will ignore gratuitous NAKs that are premised on large scale
rewrites that are out of scope for the problem.

Here the requested rework has an acceptable scope though and we can
sidestep the whole problem and solve it so everyone's happy.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
  2025-07-01  8:41       ` Christian Brauner
@ 2025-07-01  8:46         ` Qu Wenruo
  0 siblings, 0 replies; 17+ messages in thread
From: Qu Wenruo @ 2025-07-01  8:46 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Christoph Hellwig, Qu Wenruo, linux-btrfs, linux-fsdevel, viro,
	jack, linux-ext4, linux-f2fs-devel, ntfs3, linux-xfs



在 2025/7/1 18:11, Christian Brauner 写道:
> On Tue, Jul 01, 2025 at 04:05:03PM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2025/7/1 15:44, Christoph Hellwig 写道:
>>> On Tue, Jul 01, 2025 at 03:02:34PM +0930, Qu Wenruo wrote:
>>>> To allow those multi-device filesystems to be integrated to use
>>>> fs_holder_ops:
>>>>
>>>> - Rename shutdown() call back to remove_bdev()
>>>>     To better describe when the call back is called.
>>>
>>> What is renamed back here?
>>
>> Rename the old shutdown to remove_bdev().
>>
>>>
>>>> -static void exfat_shutdown(struct super_block *sb)
>>>> +static void exfat_shutdown(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_shutdown,
>>>
>>> Please also rename the function so that they match the method name.
>>
>> I prefer not, and it is intentionally left as is.
>>
>> This give us a very clear view what a fs is expected to do.
> 
> Qu, would you please rename the individual functions?

Sure. I'll keep the fs' function names consistent with the callback names.

Especially there are already quite some maintainers wanting a consistent 
pattern here.

Thanks,
Qu

> 
> The NAK later just because of this is unnecessary. I will say clearly
> that I will ignore gratuitous NAKs that are premised on large scale
> rewrites that are out of scope for the problem.
> 
> Here the requested rework has an acceptable scope though and we can
> sidestep the whole problem and solve it so everyone's happy.
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] btrfs: implement shutdown ioctl
  2025-07-01  5:32 ` [PATCH v2 5/6] btrfs: implement shutdown ioctl Qu Wenruo
@ 2025-07-02  9:38   ` kernel test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2025-07-02  9:38 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs, linux-fsdevel
  Cc: llvm, oe-kbuild-all, viro, brauner, jack

Hi Qu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on brauner-vfs/vfs.all tytso-ext4/dev jaegeuk-f2fs/dev xfs-linux/for-next linus/master v6.16-rc4 next-20250701]
[cannot apply to jaegeuk-f2fs/dev-test]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/fs-enhance-and-rename-shutdown-callback-to-remove_bdev/20250701-133555
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link:    https://lore.kernel.org/r/7572ae432f4caebf074e0b9db8a88a502aed3217.1751347436.git.wqu%40suse.com
patch subject: [PATCH v2 5/6] btrfs: implement shutdown ioctl
config: arm-randconfig-001-20250702 (https://download.01.org/0day-ci/archive/20250702/202507021710.jq2jhLL2-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250702/202507021710.jq2jhLL2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507021710.jq2jhLL2-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:5250:16: warning: unused variable 'flags' [-Wunused-variable]
    5250 |         unsigned long flags;
         |                       ^~~~~
   1 warning generated.


vim +/flags +5250 fs/btrfs/ioctl.c

  5241	
  5242	long btrfs_ioctl(struct file *file, unsigned int
  5243			cmd, unsigned long arg)
  5244	{
  5245		struct inode *inode = file_inode(file);
  5246		struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
  5247		struct btrfs_root *root = BTRFS_I(inode)->root;
  5248		void __user *argp = (void __user *)arg;
  5249		/* If @arg is just an unsigned long value. */
> 5250		unsigned long flags;
  5251	
  5252		switch (cmd) {
  5253		case FS_IOC_GETVERSION:
  5254			return btrfs_ioctl_getversion(inode, argp);
  5255		case FS_IOC_GETFSLABEL:
  5256			return btrfs_ioctl_get_fslabel(fs_info, argp);
  5257		case FS_IOC_SETFSLABEL:
  5258			return btrfs_ioctl_set_fslabel(file, argp);
  5259		case FITRIM:
  5260			return btrfs_ioctl_fitrim(fs_info, argp);
  5261		case BTRFS_IOC_SNAP_CREATE:
  5262			return btrfs_ioctl_snap_create(file, argp, 0);
  5263		case BTRFS_IOC_SNAP_CREATE_V2:
  5264			return btrfs_ioctl_snap_create_v2(file, argp, 0);
  5265		case BTRFS_IOC_SUBVOL_CREATE:
  5266			return btrfs_ioctl_snap_create(file, argp, 1);
  5267		case BTRFS_IOC_SUBVOL_CREATE_V2:
  5268			return btrfs_ioctl_snap_create_v2(file, argp, 1);
  5269		case BTRFS_IOC_SNAP_DESTROY:
  5270			return btrfs_ioctl_snap_destroy(file, argp, false);
  5271		case BTRFS_IOC_SNAP_DESTROY_V2:
  5272			return btrfs_ioctl_snap_destroy(file, argp, true);
  5273		case BTRFS_IOC_SUBVOL_GETFLAGS:
  5274			return btrfs_ioctl_subvol_getflags(BTRFS_I(inode), argp);
  5275		case BTRFS_IOC_SUBVOL_SETFLAGS:
  5276			return btrfs_ioctl_subvol_setflags(file, argp);
  5277		case BTRFS_IOC_DEFAULT_SUBVOL:
  5278			return btrfs_ioctl_default_subvol(file, argp);
  5279		case BTRFS_IOC_DEFRAG:
  5280			return btrfs_ioctl_defrag(file, NULL);
  5281		case BTRFS_IOC_DEFRAG_RANGE:
  5282			return btrfs_ioctl_defrag(file, argp);
  5283		case BTRFS_IOC_RESIZE:
  5284			return btrfs_ioctl_resize(file, argp);
  5285		case BTRFS_IOC_ADD_DEV:
  5286			return btrfs_ioctl_add_dev(fs_info, argp);
  5287		case BTRFS_IOC_RM_DEV:
  5288			return btrfs_ioctl_rm_dev(file, argp);
  5289		case BTRFS_IOC_RM_DEV_V2:
  5290			return btrfs_ioctl_rm_dev_v2(file, argp);
  5291		case BTRFS_IOC_FS_INFO:
  5292			return btrfs_ioctl_fs_info(fs_info, argp);
  5293		case BTRFS_IOC_DEV_INFO:
  5294			return btrfs_ioctl_dev_info(fs_info, argp);
  5295		case BTRFS_IOC_TREE_SEARCH:
  5296			return btrfs_ioctl_tree_search(root, argp);
  5297		case BTRFS_IOC_TREE_SEARCH_V2:
  5298			return btrfs_ioctl_tree_search_v2(root, argp);
  5299		case BTRFS_IOC_INO_LOOKUP:
  5300			return btrfs_ioctl_ino_lookup(root, argp);
  5301		case BTRFS_IOC_INO_PATHS:
  5302			return btrfs_ioctl_ino_to_path(root, argp);
  5303		case BTRFS_IOC_LOGICAL_INO:
  5304			return btrfs_ioctl_logical_to_ino(fs_info, argp, 1);
  5305		case BTRFS_IOC_LOGICAL_INO_V2:
  5306			return btrfs_ioctl_logical_to_ino(fs_info, argp, 2);
  5307		case BTRFS_IOC_SPACE_INFO:
  5308			return btrfs_ioctl_space_info(fs_info, argp);
  5309		case BTRFS_IOC_SYNC: {
  5310			int ret;
  5311	
  5312			ret = btrfs_start_delalloc_roots(fs_info, LONG_MAX, false);
  5313			if (ret)
  5314				return ret;
  5315			ret = btrfs_sync_fs(inode->i_sb, 1);
  5316			/*
  5317			 * There may be work for the cleaner kthread to do (subvolume
  5318			 * deletion, delayed iputs, defrag inodes, etc), so wake it up.
  5319			 */
  5320			wake_up_process(fs_info->cleaner_kthread);
  5321			return ret;
  5322		}
  5323		case BTRFS_IOC_START_SYNC:
  5324			return btrfs_ioctl_start_sync(root, argp);
  5325		case BTRFS_IOC_WAIT_SYNC:
  5326			return btrfs_ioctl_wait_sync(fs_info, argp);
  5327		case BTRFS_IOC_SCRUB:
  5328			return btrfs_ioctl_scrub(file, argp);
  5329		case BTRFS_IOC_SCRUB_CANCEL:
  5330			return btrfs_ioctl_scrub_cancel(fs_info);
  5331		case BTRFS_IOC_SCRUB_PROGRESS:
  5332			return btrfs_ioctl_scrub_progress(fs_info, argp);
  5333		case BTRFS_IOC_BALANCE_V2:
  5334			return btrfs_ioctl_balance(file, argp);
  5335		case BTRFS_IOC_BALANCE_CTL:
  5336			return btrfs_ioctl_balance_ctl(fs_info, arg);
  5337		case BTRFS_IOC_BALANCE_PROGRESS:
  5338			return btrfs_ioctl_balance_progress(fs_info, argp);
  5339		case BTRFS_IOC_SET_RECEIVED_SUBVOL:
  5340			return btrfs_ioctl_set_received_subvol(file, argp);
  5341	#ifdef CONFIG_64BIT
  5342		case BTRFS_IOC_SET_RECEIVED_SUBVOL_32:
  5343			return btrfs_ioctl_set_received_subvol_32(file, argp);
  5344	#endif
  5345		case BTRFS_IOC_SEND:
  5346			return _btrfs_ioctl_send(root, argp, false);
  5347	#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
  5348		case BTRFS_IOC_SEND_32:
  5349			return _btrfs_ioctl_send(root, argp, true);
  5350	#endif
  5351		case BTRFS_IOC_GET_DEV_STATS:
  5352			return btrfs_ioctl_get_dev_stats(fs_info, argp);
  5353		case BTRFS_IOC_QUOTA_CTL:
  5354			return btrfs_ioctl_quota_ctl(file, argp);
  5355		case BTRFS_IOC_QGROUP_ASSIGN:
  5356			return btrfs_ioctl_qgroup_assign(file, argp);
  5357		case BTRFS_IOC_QGROUP_CREATE:
  5358			return btrfs_ioctl_qgroup_create(file, argp);
  5359		case BTRFS_IOC_QGROUP_LIMIT:
  5360			return btrfs_ioctl_qgroup_limit(file, argp);
  5361		case BTRFS_IOC_QUOTA_RESCAN:
  5362			return btrfs_ioctl_quota_rescan(file, argp);
  5363		case BTRFS_IOC_QUOTA_RESCAN_STATUS:
  5364			return btrfs_ioctl_quota_rescan_status(fs_info, argp);
  5365		case BTRFS_IOC_QUOTA_RESCAN_WAIT:
  5366			return btrfs_ioctl_quota_rescan_wait(fs_info);
  5367		case BTRFS_IOC_DEV_REPLACE:
  5368			return btrfs_ioctl_dev_replace(fs_info, argp);
  5369		case BTRFS_IOC_GET_SUPPORTED_FEATURES:
  5370			return btrfs_ioctl_get_supported_features(argp);
  5371		case BTRFS_IOC_GET_FEATURES:
  5372			return btrfs_ioctl_get_features(fs_info, argp);
  5373		case BTRFS_IOC_SET_FEATURES:
  5374			return btrfs_ioctl_set_features(file, argp);
  5375		case BTRFS_IOC_GET_SUBVOL_INFO:
  5376			return btrfs_ioctl_get_subvol_info(inode, argp);
  5377		case BTRFS_IOC_GET_SUBVOL_ROOTREF:
  5378			return btrfs_ioctl_get_subvol_rootref(root, argp);
  5379		case BTRFS_IOC_INO_LOOKUP_USER:
  5380			return btrfs_ioctl_ino_lookup_user(file, argp);
  5381		case FS_IOC_ENABLE_VERITY:
  5382			return fsverity_ioctl_enable(file, (const void __user *)argp);
  5383		case FS_IOC_MEASURE_VERITY:
  5384			return fsverity_ioctl_measure(file, argp);
  5385		case FS_IOC_READ_VERITY_METADATA:
  5386			return fsverity_ioctl_read_metadata(file, argp);
  5387		case BTRFS_IOC_ENCODED_READ:
  5388			return btrfs_ioctl_encoded_read(file, argp, false);
  5389		case BTRFS_IOC_ENCODED_WRITE:
  5390			return btrfs_ioctl_encoded_write(file, argp, false);
  5391	#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
  5392		case BTRFS_IOC_ENCODED_READ_32:
  5393			return btrfs_ioctl_encoded_read(file, argp, true);
  5394		case BTRFS_IOC_ENCODED_WRITE_32:
  5395			return btrfs_ioctl_encoded_write(file, argp, true);
  5396	#endif
  5397		case BTRFS_IOC_SUBVOL_SYNC_WAIT:
  5398			return btrfs_ioctl_subvol_sync(fs_info, argp);
  5399	#ifdef CONFIG_BTRFS_EXPERIMENTAL
  5400		case BTRFS_IOC_SHUTDOWN:
  5401			if (!capable(CAP_SYS_ADMIN))
  5402				return -EPERM;
  5403			if (get_user(flags, (__u32 __user *)arg))
  5404				return -EFAULT;
  5405			return btrfs_emergency_shutdown(fs_info, flags);
  5406	#endif
  5407		}
  5408	
  5409		return -ENOTTY;
  5410	}
  5411	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-07-02  9:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-01  5:32 [PATCH v2 0/6] btrfs: add remove_bdev() callback Qu Wenruo
2025-07-01  5:32 ` [PATCH v2 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
2025-07-01  5:54   ` Darrick J. Wong
2025-07-01  6:14   ` Christoph Hellwig
2025-07-01  6:35     ` Qu Wenruo
2025-07-01  6:40       ` Christoph Hellwig
2025-07-01  8:41       ` Christian Brauner
2025-07-01  8:46         ` Qu Wenruo
2025-07-01  7:52   ` Jan Kara
2025-07-01  5:32 ` [PATCH v2 2/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN Qu Wenruo
2025-07-01  5:32 ` [PATCH v2 3/6] btrfs: reject file operations if in shutdown state Qu Wenruo
2025-07-01  5:32 ` [PATCH v2 4/6] btrfs: reject delalloc ranges " Qu Wenruo
2025-07-01  5:32 ` [PATCH v2 5/6] btrfs: implement shutdown ioctl Qu Wenruo
2025-07-02  9:38   ` kernel test robot
2025-07-01  5:32 ` [PATCH v2 6/6] btrfs: implement remove_bdev super operation callback Qu Wenruo
2025-07-01  8:21   ` Anand Jain
2025-07-01  8:30     ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).