Re: [GIT PULL] bcachefs

From: Kent Overstreet
Date: Mon Aug 14 2023 - 11:24:47 EST


On Mon, Aug 14, 2023 at 09:25:54AM +0200, Christian Brauner wrote:
> On Fri, Aug 11, 2023 at 08:58:01AM -0400, Kent Overstreet wrote:
> > I don't see the justification for the delay - every cycle there's some
> > amount of vfs/block layer refactoring that affects filesystems, the
> > super work is no different.
>
> So, the reason is that we're very close to having the super code
> massaged in a shape where bcachefs should be able to directly make use
> of the helpers instead of having to pull in custom code at all. But not
> all that work has made it.

Well, bcachefs really isn't doing anything terribly unusual here; we're
using sget() directly, same as btrfs, and we have to because we're both
multi device filesystems.

Jan's restructing of mount_bdev() got me thinking that it should be
possible to do a mount_bdevs() that both btrfs and bcachefs could use -
but we don't need to be blocked on that, sget()'s been a normal exported
interface since forever.

Somewhat related, I dropped this patch from my tree:
block: Don't block on s_umount from __invalidate_super()
https://evilpiepirate.org/git/bcachefs.git/commit/?h=bcachefs-v6.3&id=1dd488901bc025a61e1ce1a0f54999a2b221bd78

and instead, for now we're closing block devices later in the shutdown
path like other filesystems do (after calling generic_shutdown_super(),
not in put_super()).

But now I've got some test failures, e.g.
https://evilpiepirate.org/~testdashboard/c/040e910f7f316ea6273c895dcc026b9f1ad36a8e/xfstests.generic.604/log.br

and since you guys are switching block device opens to use a real
holder, I suspect you'll be seeing the same issue soon.

The bug is that the mount appears to be gone - generic_shutdown_super()
is finished - so as far as userspace can tell everything is shutdown and
we should be able to start using the block device again, but the unmount
path hasn't actually called blkdev_put() yet.

So that patch I posted is one way to solve the self-deadlock from
calling blkdev_put() where we really want to be calling it... not the
prettiest way, but I think this is something we do need to get fixed.