On Thu, Jun 26, 2025 at 10:38:11AM +0200, Christian Brauner wrote: > > + 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. Yes. As I mentioned before I think we just want to transition everyone to the remove_bdev API. But our life is probably easier if Qu's series only adds the new one so that we can do the transitions through the file system trees instead of needing a global API change. Or am I overthinking this? > > + * @surprse: indicates a surprise removal. If true the device/media is surprse is misspelled here. But I don't see how passing this on to the file system would be useful, because at this point everything is a surprise for the file system.