[PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Jan Kara]

Convert bcachefs to use bdev_open_by_path() and pass the handle around.

CC: Kent Overstreet <kent.overstreet@linux.dev>
CC: Brian Foster <bfoster@redhat.com>
CC: linux-bcachefs@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 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
@@ -162,8 +162,8 @@ void bch2_sb_field_delete(struct bch_sb_handle *sb,
 void bch2_free_super(struct bch_sb_handle *sb)
 {
 	kfree(sb->bio);
-	if (!IS_ERR_OR_NULL(sb->bdev))
-		blkdev_put(sb->bdev, sb->holder);
+	if (!IS_ERR_OR_NULL(sb->bdev_handle))
+		bdev_release(sb->bdev_handle);
 	kfree(sb->holder);
 
 	kfree(sb->sb);
@@ -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) {
diff --git a/fs/bcachefs/super_types.h b/fs/bcachefs/super_types.h
index 78d6138db62d..b77d8897c9fa 100644
--- a/fs/bcachefs/super_types.h
+++ b/fs/bcachefs/super_types.h
@@ -4,6 +4,7 @@
 
 struct bch_sb_handle {
 	struct bch_sb		*sb;
+	struct bdev_handle	*bdev_handle;
 	struct block_device	*bdev;
 	struct bio		*bio;
 	void			*holder;
-- 
2.35.3

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Brian Foster]

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 <kent.overstreet@linux.dev>
> CC: Brian Foster <bfoster@redhat.com>
> CC: linux-bcachefs@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  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...

Brian

> diff --git a/fs/bcachefs/super_types.h b/fs/bcachefs/super_types.h
> index 78d6138db62d..b77d8897c9fa 100644
> --- a/fs/bcachefs/super_types.h
> +++ b/fs/bcachefs/super_types.h
> @@ -4,6 +4,7 @@
>  
>  struct bch_sb_handle {
>  	struct bch_sb		*sb;
> +	struct bdev_handle	*bdev_handle;
>  	struct block_device	*bdev;
>  	struct bio		*bio;
>  	void			*holder;
> -- 
> 2.35.3
> 

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Kent Overstreet]

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 <kent.overstreet@linux.dev>
> > CC: Brian Foster <bfoster@redhat.com>
> > CC: linux-bcachefs@vger.kernel.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  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 :)

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Jan Kara]

Hi Brian,

On Wed 01-11-23 15:01:22, 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 <kent.overstreet@linux.dev>
> > CC: Brian Foster <bfoster@redhat.com>
> > CC: linux-bcachefs@vger.kernel.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  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) {
> 
> 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...

Yes, so AFAICS bcachefs will not get the write restriction from the changes
in the generic code. Using sb_open_mode() in bcachefs would fix that but
given the 'noexcl' and 'nochanges' mount options it will not be completely
seamless anyway. I guess once the generic changes are in, bcachefs can
decide how exactly it wants to set the BLK_OPEN_RESTRICT_WRITES flag. Or if
you already have an opinion, we can just add the patch to this series.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Brian Foster]

On Thu, Nov 02, 2023 at 10:55:50AM +0100, Jan Kara wrote:
> Hi Brian,
> 
> On Wed 01-11-23 15:01:22, 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 <kent.overstreet@linux.dev>
> > > CC: Brian Foster <bfoster@redhat.com>
> > > CC: linux-bcachefs@vger.kernel.org
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > ---
> > >  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) {
> > 
> > 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...
> 
> Yes, so AFAICS bcachefs will not get the write restriction from the changes
> in the generic code. Using sb_open_mode() in bcachefs would fix that but
> given the 'noexcl' and 'nochanges' mount options it will not be completely
> seamless anyway. I guess once the generic changes are in, bcachefs can
> decide how exactly it wants to set the BLK_OPEN_RESTRICT_WRITES flag. Or if
> you already have an opinion, we can just add the patch to this series.
> 

Yep, makes sense, and I agree with Kent this should be a separate patch
anyways. Just wanted to make sure I wasn't missing something or
otherwise it was known that bcachefs needs a bit more work to turn this
on... thanks.

Brian

> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> 

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Kent Overstreet]

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 <kent.overstreet@linux.dev>
> CC: Brian Foster <bfoster@redhat.com>
> CC: linux-bcachefs@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Kent Overstreet <kent.overstreet@linux.dev>

Re: [PATCH 1/7] bcachefs: Convert to bdev_open_by_path()

[Christian Brauner]

On Wed, 1 Nov 2023 18:43:06 +0100, Jan Kara wrote:
> Convert bcachefs to use bdev_open_by_path() and pass the handle around.
> 
> 

Applied to the vfs.super branch of the vfs/vfs.git tree.
Patches in the vfs.super branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.super

[1/7] bcachefs: Convert to bdev_open_by_path()
      https://git.kernel.org/vfs/vfs/c/8e897399352c
[2/7] block: Remove blkdev_get_by_*() functions
      https://git.kernel.org/vfs/vfs/c/1dc2789bf2d9
[3/7] block: Add config option to not allow writing to mounted devices
      https://git.kernel.org/vfs/vfs/c/708e8ecda49e
[4/7] btrfs: Do not restrict writes to btrfs devices
      https://git.kernel.org/vfs/vfs/c/b6b2f4843264
[5/7] fs: Block writes to mounted block devices
      https://git.kernel.org/vfs/vfs/c/48ce483465bb
[6/7] xfs: Block writes to log device
      https://git.kernel.org/vfs/vfs/c/dae1e956882c
[7/7] ext4: Block writes to journal device
      https://git.kernel.org/vfs/vfs/c/a8a97da12393