Re: [PATCH 1/6] fs: add a new remove_bdev() super operations callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2025/6/26 18:40, Christian Brauner 写道:
On Thu, Jun 26, 2025 at 06:14:03PM +0930, Qu Wenruo wrote:


在 2025/6/26 18:08, Christian Brauner 写道:
On Thu, Jun 26, 2025 at 09:23:42AM +0930, Qu Wenruo wrote:
The new remove_bdev() call back is mostly for multi-device filesystems
to handle device removal.

Some multi-devices filesystems like btrfs can have the ability to handle
device lose according to the setup (e.g. all chunks have extra mirrors),
thus losing a block device will not interrupt the normal operations.

Btrfs will soon implement this call back by:

- Automatically degrade the fs if read-write operations can be
    maintained

- Shutdown the fs if read-write operations can not be maintained

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

diff --git a/fs/super.c b/fs/super.c
index 80418ca8e215..07845d2f9ec4 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1463,7 +1463,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->remove_bdev)
+		sb->s_op->remove_bdev(sb, bdev, surprise);
+	else if (sb->s_op->shutdown)
   		sb->s_op->shutdown(sb);

This makes ->remove_bdev() and ->shutdown() mutually exclusive. I really
really dislike this pattern. It introduces the possibility that a
filesystem accidently implement both variants and assumes both are
somehow called. That can be solved by an assert at superblock initation
time but it's still nasty.

The other thing is that this just reeks of being the wrong api. We
should absolutely aim for the methods to not be mutually exclusive. I
hate that with a passion. That's just an ugly api and I want to have as
little of that as possible in our code.

So what do you really want?

The original path to expand the shutdown() callback is rejected, and now the
new callback is also rejected.

I guess the only thing left is to rename shutdown() to remove_bdev(), add
the new parameters and keep existing fses doing what they do (aka,
shutdown)?

Yes. My original understanding had been that ->remove_bdev() would be
called in a different codepath than ->shutdown() and they would be
complimentary. So sorry for the back and forth here. If that's not the
case I don't see any point in having two distinct methods.

That's fine, just want to do a final confirmation that everyone is fine with such change:

diff --git a/include/linux/fs.h b/include/linux/fs.h
index b085f161ed22..c11b9219863b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2367,7 +2367,8 @@ struct super_operations {
                                  struct shrink_control *);
        long (*free_cached_objects)(struct super_block *,
                                    struct shrink_control *);
-       void (*shutdown)(struct super_block *sb);
+ void (*remove_bdev)(struct super_block *sb, struct block_device *bdev,
+                           bool surprise);
 };

 /*




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux