Re: [PATCH] fs: Avoid grabbing sb->s_umount under bdev->bd_holder_lock

[Christian Brauner <brauner@kernel.org>]

On Thu, Oct 19, 2023 at 12:57:17PM +0200, Jan Kara wrote:
> On Thu 19-10-23 10:33:36, Christian Brauner wrote:
> > On Wed, Oct 18, 2023 at 05:29:24PM +0200, Jan Kara wrote:
> > > The implementation of bdev holder operations such as fs_bdev_mark_dead()
> > > and fs_bdev_sync() grab sb->s_umount semaphore under
> > > bdev->bd_holder_lock. This is problematic because it leads to
> > > disk->open_mutex -> sb->s_umount lock ordering which is counterintuitive
> > > (usually we grab higher level (e.g. filesystem) locks first and lower
> > > level (e.g. block layer) locks later) and indeed makes lockdep complain
> > > about possible locking cycles whenever we open a block device while
> > > holding sb->s_umount semaphore. Implement a function
> > 
> > This patches together with my series that Christoph sent out for me
> > Link: https://lore.kernel.org/r/20231017184823.1383356-1-hch@lst.de
> > two days ago (tyvm!) the lockdep false positives are all gone and we
> > also eliminated the counterintuitive ordering requirement that forces us
> > to give up s_umount before opening block devices.
> > 
> > I've verified that yesterday and did a bunch of testing via sudden
> > device removal.
> > 
> > Christoph had thankfully added generic/730 and generic/731 to emulate
> > some device removal. I also messed around with the loop code and
> > specifically used LOOP_CHANGE_FD to force a disk_force_media_change() on
> > a filesystem.
> 
> Ah, glad to hear that! So now we can also slowly work on undoing the unlock
> s_umount, open bdev, lock s_umount games we have introduced in several
> places. But I guess let's wait a bit for the dust to settle :)

Yeah, I had thought about this right away as well. And agreed, that can
happen later. :)