On Wed, Nov 01, 2023 at 03:01:22PM -0400, Brian Foster wrote: > On Wed, Nov 01, 2023 at 06:43:06PM +0100, Jan Kara wrote: > > Convert bcachefs to use bdev_open_by_path() and pass the handle around. > > > > CC: Kent Overstreet <[email protected]> > > CC: Brian Foster <[email protected]> > > CC: [email protected] > > Signed-off-by: Jan Kara <[email protected]> > > --- > > fs/bcachefs/super-io.c | 19 ++++++++++--------- > > fs/bcachefs/super_types.h | 1 + > > 2 files changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/fs/bcachefs/super-io.c b/fs/bcachefs/super-io.c > > index 332d41e1c0a3..01a32c41a540 100644 > > --- a/fs/bcachefs/super-io.c > > +++ b/fs/bcachefs/super-io.c > ... > > @@ -685,21 +685,22 @@ int bch2_read_super(const char *path, struct bch_opts > > *opts, > > if (!opt_get(*opts, nochanges)) > > sb->mode |= BLK_OPEN_WRITE; > > > > - sb->bdev = blkdev_get_by_path(path, sb->mode, sb->holder, > > &bch2_sb_handle_bdev_ops); > > - if (IS_ERR(sb->bdev) && > > - PTR_ERR(sb->bdev) == -EACCES && > > + sb->bdev_handle = bdev_open_by_path(path, sb->mode, sb->holder, > > &bch2_sb_handle_bdev_ops); > > + if (IS_ERR(sb->bdev_handle) && > > + PTR_ERR(sb->bdev_handle) == -EACCES && > > opt_get(*opts, read_only)) { > > sb->mode &= ~BLK_OPEN_WRITE; > > > > - sb->bdev = blkdev_get_by_path(path, sb->mode, sb->holder, > > &bch2_sb_handle_bdev_ops); > > - if (!IS_ERR(sb->bdev)) > > + sb->bdev_handle = bdev_open_by_path(path, sb->mode, sb->holder, > > &bch2_sb_handle_bdev_ops); > > + if (!IS_ERR(sb->bdev_handle)) > > opt_set(*opts, nochanges, true); > > } > > > > - if (IS_ERR(sb->bdev)) { > > - ret = PTR_ERR(sb->bdev); > > + if (IS_ERR(sb->bdev_handle)) { > > + ret = PTR_ERR(sb->bdev_handle); > > goto out; > > } > > + sb->bdev = sb->bdev_handle->bdev; > > > > ret = bch2_sb_realloc(sb, 0); > > if (ret) { > > Hi Jan, > > This all seems reasonable to me, but should bcachefs use sb_open_mode() > somewhere in here to involve use of the restrict writes flag in the > first place? It looks like bcachefs sort of open codes bits of > mount_bdev() so it isn't clear the flag would be used anywhere...
Yeah, but that should be a separate patch :)
