linux-kernel - Re: [GIT PULL] bcachefs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230814152335.oiroowt2co2kj4xx@moria.home.lan>
Date:   Mon, 14 Aug 2023 11:23:35 -0400
From:   Kent Overstreet <kent.overstreet@...ux.dev>
To:     Christian Brauner <brauner@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-bcachefs@...r.kernel.org, djwong@...nel.org,
        dchinner@...hat.com, sandeen@...hat.com, willy@...radead.org,
        josef@...icpanda.com, tytso@....edu, bfoster@...hat.com,
        jack@...e.cz, andreas.gruenbacher@...il.com, peterz@...radead.org,
        akpm@...ux-foundation.org, dhowells@...hat.com, snitzer@...nel.org,
        axboe@...nel.dk
Subject: Re: [GIT PULL] bcachefs

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.