From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Qu Wenruo <quwenruo.btrfs@gmx.com>,
Dave Chinner <david@fromorbit.com>, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz,
linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net, ntfs3@lists.linux.dev,
linux-xfs@vger.kernel.org
Subject: Re: [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
Date: Thu, 10 Jul 2025 03:52:16 -0700 [thread overview]
Message-ID: <aG-bYCFix5lcPyqg@infradead.org> (raw)
In-Reply-To: <20250708004532.GA2672018@frogsfrogsfrogs>
On Mon, Jul 07, 2025 at 05:45:32PM -0700, Darrick J. Wong wrote:
> On second thought -- it's weird that you'd flush the filesystem and
> shrink the inode/dentry caches in a "your device went away" handler.
> Fancy filesystems like bcachefs and btrfs would likely just shift IO to
> a different bdev, right? And there's no good reason to run shrinkers on
> either of those fses, right?
No nmeed for fancy file systems, this is weird no matter what. But it
is what Linux has done for 30+ years, so I kept it when refactoring
this code to sit in a callback.
> > Yes, the naming is not perfect and mixing cause and action, but the end
> > result is still a more generic and less duplicated code base.
>
> I think dchinner makes a good point that if your filesystem can do
> something clever on device removal, it should provide its own block
> device holder ops instead of using fs_holder_ops. I don't understand
> why you need a "generic" solution for btrfs when it's not going to do
> what the others do anyway.
Why? You're most likely to get the locking wrong, and so on.
What might make sense is to move the sync_filesystem, shrink_dcache_sb
and evict_inodes into the method. That way file systems where we
> As an aside:
> 'twould be nice if we could lift the *FS_IOC_SHUTDOWN dispatch out of
> everyone's ioctl functions into the VFS, and then move the "I am dead"
> state into super_block so that you could actually shut down any
> filesystem, not just the seven that currently implement it.
Sure. Someone just needs to do the work..
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: brauner@kernel.org, ntfs3@lists.linux.dev, jack@suse.cz,
Dave Chinner <david@fromorbit.com>,
Qu Wenruo <quwenruo.btrfs@gmx.com>,
linux-f2fs-devel@lists.sourceforge.net,
linux-xfs@vger.kernel.org, Qu Wenruo <wqu@suse.com>,
linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-btrfs@vger.kernel.org, viro@zeniv.linux.org.uk
Subject: Re: [f2fs-dev] [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev()
Date: Thu, 10 Jul 2025 03:52:16 -0700 [thread overview]
Message-ID: <aG-bYCFix5lcPyqg@infradead.org> (raw)
In-Reply-To: <20250708004532.GA2672018@frogsfrogsfrogs>
On Mon, Jul 07, 2025 at 05:45:32PM -0700, Darrick J. Wong wrote:
> On second thought -- it's weird that you'd flush the filesystem and
> shrink the inode/dentry caches in a "your device went away" handler.
> Fancy filesystems like bcachefs and btrfs would likely just shift IO to
> a different bdev, right? And there's no good reason to run shrinkers on
> either of those fses, right?
No nmeed for fancy file systems, this is weird no matter what. But it
is what Linux has done for 30+ years, so I kept it when refactoring
this code to sit in a callback.
> > Yes, the naming is not perfect and mixing cause and action, but the end
> > result is still a more generic and less duplicated code base.
>
> I think dchinner makes a good point that if your filesystem can do
> something clever on device removal, it should provide its own block
> device holder ops instead of using fs_holder_ops. I don't understand
> why you need a "generic" solution for btrfs when it's not going to do
> what the others do anyway.
Why? You're most likely to get the locking wrong, and so on.
What might make sense is to move the sync_filesystem, shrink_dcache_sb
and evict_inodes into the method. That way file systems where we
> As an aside:
> 'twould be nice if we could lift the *FS_IOC_SHUTDOWN dispatch out of
> everyone's ioctl functions into the VFS, and then move the "I am dead"
> state into super_block so that you could actually shut down any
> filesystem, not just the seven that currently implement it.
Sure. Someone just needs to do the work..
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2025-07-10 10:52 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-04 0:42 [PATCH v4 0/6] btrfs: add remove_bdev() callback Qu Wenruo
2025-07-04 0:42 ` [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Qu Wenruo
2025-07-04 0:42 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-04 9:00 ` (subset) " Christian Brauner
2025-07-04 9:00 ` [f2fs-dev] " Christian Brauner via Linux-f2fs-devel
2025-07-04 9:05 ` Jan Kara
2025-07-04 9:05 ` [f2fs-dev] " Jan Kara
2025-07-07 23:02 ` Dave Chinner
2025-07-07 23:02 ` [f2fs-dev] " Dave Chinner via Linux-f2fs-devel
2025-07-07 23:22 ` Qu Wenruo
2025-07-07 23:22 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-08 0:45 ` Darrick J. Wong
2025-07-08 0:45 ` [f2fs-dev] " Darrick J. Wong via Linux-f2fs-devel
2025-07-08 2:09 ` Qu Wenruo
2025-07-08 2:09 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-08 3:06 ` Qu Wenruo
2025-07-08 3:06 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-08 5:05 ` Dave Chinner
2025-07-08 5:05 ` [f2fs-dev] " Dave Chinner via Linux-f2fs-devel
2025-07-08 5:41 ` Qu Wenruo
2025-07-08 5:41 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-08 7:55 ` Christian Brauner
2025-07-08 7:55 ` [f2fs-dev] " Christian Brauner via Linux-f2fs-devel
2025-07-08 22:59 ` Dave Chinner
2025-07-08 22:59 ` [f2fs-dev] " Dave Chinner via Linux-f2fs-devel
2025-07-08 23:07 ` Qu Wenruo
2025-07-08 23:07 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-09 0:35 ` Kent Overstreet
2025-07-09 0:35 ` [f2fs-dev] " Kent Overstreet
2025-07-09 0:55 ` Qu Wenruo
2025-07-09 0:55 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-09 1:13 ` Kent Overstreet
2025-07-09 1:13 ` [f2fs-dev] " Kent Overstreet
2025-07-10 8:33 ` Christian Brauner
2025-07-10 8:33 ` [f2fs-dev] " Christian Brauner via Linux-f2fs-devel
2025-07-10 10:54 ` Christoph Hellwig
2025-07-10 10:54 ` [f2fs-dev] " Christoph Hellwig
2025-07-08 10:20 ` Jan Kara
2025-07-08 10:20 ` [f2fs-dev] " Jan Kara
2025-07-08 20:20 ` Darrick J. Wong
2025-07-08 20:20 ` [f2fs-dev] " Darrick J. Wong via Linux-f2fs-devel
2025-07-08 22:12 ` Qu Wenruo
2025-07-08 22:12 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-10 8:40 ` Christian Brauner
2025-07-10 8:40 ` [f2fs-dev] " Christian Brauner via Linux-f2fs-devel
2025-07-10 9:54 ` Qu Wenruo
2025-07-10 9:54 ` [f2fs-dev] " Qu Wenruo via Linux-f2fs-devel
2025-07-11 9:34 ` Christian Brauner
2025-07-11 9:34 ` [f2fs-dev] " Christian Brauner via Linux-f2fs-devel
2025-07-10 10:52 ` Christoph Hellwig [this message]
2025-07-10 10:52 ` Christoph Hellwig
2025-07-04 0:42 ` [PATCH v4 2/6] btrfs: introduce a new fs state, EMERGENCY_SHUTDOWN Qu Wenruo
2025-07-04 0:42 ` [PATCH v4 3/6] btrfs: reject file operations if in shutdown state Qu Wenruo
2025-07-05 14:10 ` David Sterba
2025-07-04 0:42 ` [PATCH v4 4/6] btrfs: reject delalloc ranges " Qu Wenruo
2025-07-04 0:42 ` [PATCH v4 5/6] btrfs: implement shutdown ioctl Qu Wenruo
2025-07-05 14:22 ` David Sterba
2025-07-06 3:37 ` Qu Wenruo
2025-07-07 20:51 ` David Sterba
2025-07-07 23:04 ` Qu Wenruo
2025-07-08 0:53 ` David Sterba
2025-07-04 0:42 ` [PATCH v4 6/6] btrfs: implement remove_bdev super operation callback Qu Wenruo
-- strict thread matches above, loose matches on Subject: below --
2025-07-09 17:23 [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev() Jan Kara
2025-07-09 17:49 ` Kent Overstreet
2025-07-10 13:10 ` Jan Kara
2025-07-10 18:41 ` Kent Overstreet
2025-07-11 14:20 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aG-bYCFix5lcPyqg@infradead.org \
--to=hch@infradead.org \
--cc=brauner@kernel.org \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=jack@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=ntfs3@lists.linux.dev \
--cc=quwenruo.btrfs@gmx.com \
--cc=viro@zeniv.linux.org.uk \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.