diff mbox series

[v2,29/34] bdev: make struct bdev_handle private to the block layer

Message ID 20240123-vfs-bdev-file-v2-29-adbd023e19cc@kernel.org (mailing list archive)
State New, archived
Headers show
Series Open block devices as files | expand

Commit Message

Christian Brauner Jan. 23, 2024, 1:26 p.m. UTC
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 block/bdev.c           | 125 +++++++++++++++++++++++++++----------------------
 block/blk.h            |  12 +++--
 block/fops.c           |  34 ++++++--------
 include/linux/blkdev.h |   7 ---
 include/linux/fs.h     |   6 ---
 5 files changed, 92 insertions(+), 92 deletions(-)

Comments

Christoph Hellwig Jan. 29, 2024, 4:22 p.m. UTC | #1
> +	ret = devcgroup_check_permission(
> +		DEVCG_DEV_BLOCK, MAJOR(dev), MINOR(dev),
> +		((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
> +			((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));

Somewhat weird formatting here with DEVCG_DEV_BLOCK not on the
same line as the opening brace and the extra indentation after
the |.  I would have expected something like:

	ret = devcgroup_check_permission(DEVCG_DEV_BLOCK,
		MAJOR(dev), MINOR(dev),
		((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
		((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jan Kara Feb. 1, 2024, 10:54 a.m. UTC | #2
On Tue 23-01-24 14:26:46, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Some comments below...

> @@ -795,15 +813,15 @@ static void bdev_yield_write_access(struct block_device *bdev, blk_mode_t mode)
>  }
>  
>  /**
> - * bdev_open_by_dev - open a block device by device number
> - * @dev: device number of block device to open
> + * bdev_open - open a block device
> + * @bdev: block device to open
>   * @mode: open mode (BLK_OPEN_*)
>   * @holder: exclusive holder identifier
>   * @hops: holder operations
> + * @bdev_file: file for the block device
>   *
> - * Open the block device described by device number @dev. If @holder is not
> - * %NULL, the block device is opened with exclusive access.  Exclusive opens may
> - * nest for the same @holder.
> + * Open the block device. If @holder is not %NULL, the block device is opened
> + * with exclusive access.  Exclusive opens may nest for the same @holder.
>   *
>   * Use this interface ONLY if you really do not have anything better - i.e. when
>   * you are behind a truly sucky interface and all you are given is a device
      ^^^
I guess this part of comment is stale now?

> @@ -902,7 +897,22 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
>  	handle->bdev = bdev;
>  	handle->holder = holder;
>  	handle->mode = mode;
> -	return handle;
> +
> +	/*
> +	 * Preserve backwards compatibility and allow large file access
> +	 * even if userspace doesn't ask for it explicitly. Some mkfs
> +	 * binary needs it. We might want to drop this workaround
> +	 * during an unstable branch.
> +	 */

Heh, I think the sentense "We might want to drop this workaround during an
unstable branch." does not need to be moved as well :)

> +	bdev_file->f_flags |= O_LARGEFILE;
> +	bdev_file->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
> +	if (bdev_nowait(bdev))
> +		bdev_file->f_mode |= FMODE_NOWAIT;
> +	bdev_file->f_mapping = handle->bdev->bd_inode->i_mapping;
> +	bdev_file->f_wb_err = filemap_sample_wb_err(bdev_file->f_mapping);
> +	bdev_file->private_data = handle;
> +
> +	return 0;
>  put_module:
>  	module_put(disk->fops->owner);
>  abort_claiming:
> @@ -910,11 +920,8 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
>  		bd_abort_claiming(bdev, holder);
>  	mutex_unlock(&disk->open_mutex);
>  	disk_unblock_events(disk);
> -put_blkdev:
> -	blkdev_put_no_open(bdev);
> -free_handle:
>  	kfree(handle);
> -	return ERR_PTR(ret);
> +	return ret;
>  }
>  
>  static unsigned blk_to_file_flags(blk_mode_t mode)
> @@ -954,29 +961,35 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
>  				   const struct blk_holder_ops *hops)
>  {
>  	struct file *bdev_file;
> -	struct bdev_handle *handle;
> +	struct block_device *bdev;
>  	unsigned int flags;
> +	int ret;
>  
> -	handle = bdev_open_by_dev(dev, mode, holder, hops);
> -	if (IS_ERR(handle))
> -		return ERR_CAST(handle);
> +	ret = bdev_permission(dev, 0, holder);
				   ^^ Maybe I'm missing something but why
do you pass 0 mode here?


> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	bdev = blkdev_get_no_open(dev);
> +	if (!bdev)
> +		return ERR_PTR(-ENXIO);
>  
>  	flags = blk_to_file_flags(mode);
> -	bdev_file = alloc_file_pseudo_noaccount(handle->bdev->bd_inode,
> +	bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
>  			blockdev_mnt, "", flags | O_LARGEFILE, &def_blk_fops);
>  	if (IS_ERR(bdev_file)) {
> -		bdev_release(handle);
> +		blkdev_put_no_open(bdev);
>  		return bdev_file;
>  	}
> -	ihold(handle->bdev->bd_inode);
> +	bdev_file->f_mode &= ~FMODE_OPENED;

Hum, why do you need these games with FMODE_OPENED? I suspect you want to
influence fput() behavior but then AFAICT we will leak dentry, mnt, etc. on
error? If this is indeed needed, it deserves a comment...

> -	bdev_file->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
> -	if (bdev_nowait(handle->bdev))
> -		bdev_file->f_mode |= FMODE_NOWAIT;
> -
> -	bdev_file->f_mapping = handle->bdev->bd_inode->i_mapping;
> -	bdev_file->f_wb_err = filemap_sample_wb_err(bdev_file->f_mapping);
> -	bdev_file->private_data = handle;
> +	ihold(bdev->bd_inode);
> +	ret = bdev_open(bdev, mode, holder, hops, bdev_file);
> +	if (ret) {
> +		fput(bdev_file);
> +		return ERR_PTR(ret);
> +	}
> +	/* Now that thing is opened. */
> +	bdev_file->f_mode |= FMODE_OPENED;
>  	return bdev_file;
>  }
>  EXPORT_SYMBOL(bdev_file_open_by_dev);

								Honza
Jan Kara Feb. 1, 2024, 11:23 a.m. UTC | #3
On Tue 23-01-24 14:26:46, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>

One more thing I've noticed:

> -struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> -				     const struct blk_holder_ops *hops)
> +int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
> +	      const struct blk_holder_ops *hops, struct file *bdev_file)
>  {
>  	struct bdev_handle *handle = kmalloc(sizeof(struct bdev_handle),
>  					     GFP_KERNEL);
> -	struct block_device *bdev;
>  	bool unblock_events = true;
> -	struct gendisk *disk;
> +	struct gendisk *disk = bdev->bd_disk;
>  	int ret;
>  
> +	handle = kmalloc(sizeof(struct bdev_handle), GFP_KERNEL);

You are leaking handle here. It gets fixed up later in the series but
still...

								Honza
Christian Brauner Feb. 1, 2024, 2:50 p.m. UTC | #4
On Mon, Jan 29, 2024 at 05:22:03PM +0100, Christoph Hellwig wrote:
> > +	ret = devcgroup_check_permission(
> > +		DEVCG_DEV_BLOCK, MAJOR(dev), MINOR(dev),
> > +		((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
> > +			((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));
> 
> Somewhat weird formatting here with DEVCG_DEV_BLOCK not on the
> same line as the opening brace and the extra indentation after
> the |.  I would have expected something like:
> 
> 	ret = devcgroup_check_permission(DEVCG_DEV_BLOCK,
> 		MAJOR(dev), MINOR(dev),
> 		((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
> 		((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));

Fixed. (Fwiw, this is due to clang-format.)
Christian Brauner Feb. 1, 2024, 2:52 p.m. UTC | #5
On Thu, Feb 01, 2024 at 12:23:47PM +0100, Jan Kara wrote:
> On Tue 23-01-24 14:26:46, Christian Brauner wrote:
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> One more thing I've noticed:
> 
> > -struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> > -				     const struct blk_holder_ops *hops)
> > +int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
> > +	      const struct blk_holder_ops *hops, struct file *bdev_file)
> >  {
> >  	struct bdev_handle *handle = kmalloc(sizeof(struct bdev_handle),
> >  					     GFP_KERNEL);
> > -	struct block_device *bdev;
> >  	bool unblock_events = true;
> > -	struct gendisk *disk;
> > +	struct gendisk *disk = bdev->bd_disk;
> >  	int ret;
> >  
> > +	handle = kmalloc(sizeof(struct bdev_handle), GFP_KERNEL);
> 
> You are leaking handle here. It gets fixed up later in the series but
> still...

Bah, called twice instead of removed it. Fixed.
Christian Brauner Feb. 1, 2024, 3:07 p.m. UTC | #6
> >   * Use this interface ONLY if you really do not have anything better - i.e. when
> >   * you are behind a truly sucky interface and all you are given is a device
>       ^^^
> I guess this part of comment is stale now?

Indeed, I removed that.

> 
> > @@ -902,7 +897,22 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >  	handle->bdev = bdev;
> >  	handle->holder = holder;
> >  	handle->mode = mode;
> > -	return handle;
> > +
> > +	/*
> > +	 * Preserve backwards compatibility and allow large file access
> > +	 * even if userspace doesn't ask for it explicitly. Some mkfs
> > +	 * binary needs it. We might want to drop this workaround
> > +	 * during an unstable branch.
> > +	 */
> 
> Heh, I think the sentense "We might want to drop this workaround during an
> unstable branch." does not need to be moved as well :)

Dropped.

> > -	handle = bdev_open_by_dev(dev, mode, holder, hops);
> > -	if (IS_ERR(handle))
> > -		return ERR_CAST(handle);
> > +	ret = bdev_permission(dev, 0, holder);
> 				   ^^ Maybe I'm missing something but why
> do you pass 0 mode here?

Lack of caffeine? Fixed. Thanks for catching that.

> 
> 
> > +	if (ret)
> > +		return ERR_PTR(ret);
> > +
> > +	bdev = blkdev_get_no_open(dev);
> > +	if (!bdev)
> > +		return ERR_PTR(-ENXIO);
> >  
> >  	flags = blk_to_file_flags(mode);
> > -	bdev_file = alloc_file_pseudo_noaccount(handle->bdev->bd_inode,
> > +	bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
> >  			blockdev_mnt, "", flags | O_LARGEFILE, &def_blk_fops);
> >  	if (IS_ERR(bdev_file)) {
> > -		bdev_release(handle);
> > +		blkdev_put_no_open(bdev);
> >  		return bdev_file;
> >  	}
> > -	ihold(handle->bdev->bd_inode);
> > +	bdev_file->f_mode &= ~FMODE_OPENED;
> 
> Hum, why do you need these games with FMODE_OPENED? I suspect you want to
> influence fput() behavior but then AFAICT we will leak dentry, mnt, etc. on
> error? If this is indeed needed, it deserves a comment...

I rewrote this.

Total diff I applied is:

diff --git a/block/bdev.c b/block/bdev.c
index 0e8984884236..ba9dfa4648ca 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -893,12 +893,6 @@ int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
        handle->holder = holder;
        handle->mode = mode;

-       /*
-        * Preserve backwards compatibility and allow large file access
-        * even if userspace doesn't ask for it explicitly. Some mkfs
-        * binary needs it. We might want to drop this workaround
-        * during an unstable branch.
-        */
        bdev_file->f_flags |= O_LARGEFILE;
        bdev_file->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
        if (bdev_nowait(bdev))
@@ -960,7 +954,7 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
        unsigned int flags;
        int ret;

-       ret = bdev_permission(dev, 0, holder);
+       ret = bdev_permission(dev, mode, holder);
        if (ret)
                return ERR_PTR(ret);

@@ -975,16 +969,14 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
                blkdev_put_no_open(bdev);
                return bdev_file;
        }
-       bdev_file->f_mode &= ~FMODE_OPENED;
-
        ihold(bdev->bd_inode);
+
        ret = bdev_open(bdev, mode, holder, hops, bdev_file);
        if (ret) {
+               blkdev_put_no_open(bdev);
                fput(bdev_file);
                return ERR_PTR(ret);
        }
-       /* Now that thing is opened. */
-       bdev_file->f_mode |= FMODE_OPENED;
        return bdev_file;
 }
 EXPORT_SYMBOL(bdev_file_open_by_dev);
diff --git a/block/fops.c b/block/fops.c
index 81ff8c0ce32f..a1ba1a50ae77 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -622,7 +622,8 @@ static int blkdev_open(struct inode *inode, struct file *filp)

 static int blkdev_release(struct inode *inode, struct file *filp)
 {
-       bdev_release(filp->private_data);
+       if (filp->private_data)
+               bdev_release(filp->private_data);
        return 0;
 }
Jan Kara Feb. 1, 2024, 5:42 p.m. UTC | #7
On Thu 01-02-24 16:07:59, Christian Brauner wrote:
> > > +	if (ret)
> > > +		return ERR_PTR(ret);
> > > +
> > > +	bdev = blkdev_get_no_open(dev);
> > > +	if (!bdev)
> > > +		return ERR_PTR(-ENXIO);
> > >  
> > >  	flags = blk_to_file_flags(mode);
> > > -	bdev_file = alloc_file_pseudo_noaccount(handle->bdev->bd_inode,
> > > +	bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
> > >  			blockdev_mnt, "", flags | O_LARGEFILE, &def_blk_fops);
> > >  	if (IS_ERR(bdev_file)) {
> > > -		bdev_release(handle);
> > > +		blkdev_put_no_open(bdev);
> > >  		return bdev_file;
> > >  	}
> > > -	ihold(handle->bdev->bd_inode);
> > > +	bdev_file->f_mode &= ~FMODE_OPENED;
> > 
> > Hum, why do you need these games with FMODE_OPENED? I suspect you want to
> > influence fput() behavior but then AFAICT we will leak dentry, mnt, etc. on
> > error? If this is indeed needed, it deserves a comment...
> 
> I rewrote this.
> 
> Total diff I applied is:

Looks good now.

								Honza
diff mbox series

Patch

diff --git a/block/bdev.c b/block/bdev.c
index 1f64f213c5fa..34b9a16edb6e 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -703,6 +703,24 @@  static int blkdev_get_part(struct block_device *part, blk_mode_t mode)
 	return ret;
 }
 
+int bdev_permission(dev_t dev, blk_mode_t mode, void *holder)
+{
+	int ret;
+
+	ret = devcgroup_check_permission(
+		DEVCG_DEV_BLOCK, MAJOR(dev), MINOR(dev),
+		((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
+			((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));
+	if (ret)
+		return ret;
+
+	/* Blocking writes requires exclusive opener */
+	if (mode & BLK_OPEN_RESTRICT_WRITES && !holder)
+		return -EINVAL;
+
+	return 0;
+}
+
 static void blkdev_put_part(struct block_device *part)
 {
 	struct block_device *whole = bdev_whole(part);
@@ -795,15 +813,15 @@  static void bdev_yield_write_access(struct block_device *bdev, blk_mode_t mode)
 }
 
 /**
- * bdev_open_by_dev - open a block device by device number
- * @dev: device number of block device to open
+ * bdev_open - open a block device
+ * @bdev: block device to open
  * @mode: open mode (BLK_OPEN_*)
  * @holder: exclusive holder identifier
  * @hops: holder operations
+ * @bdev_file: file for the block device
  *
- * Open the block device described by device number @dev. If @holder is not
- * %NULL, the block device is opened with exclusive access.  Exclusive opens may
- * nest for the same @holder.
+ * Open the block device. If @holder is not %NULL, the block device is opened
+ * with exclusive access.  Exclusive opens may nest for the same @holder.
  *
  * Use this interface ONLY if you really do not have anything better - i.e. when
  * you are behind a truly sucky interface and all you are given is a device
@@ -813,52 +831,29 @@  static void bdev_yield_write_access(struct block_device *bdev, blk_mode_t mode)
  * Might sleep.
  *
  * RETURNS:
- * Handle with a reference to the block_device on success, ERR_PTR(-errno) on
- * failure.
+ * zero on success, -errno on failure.
  */
-struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
-				     const struct blk_holder_ops *hops)
+int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
+	      const struct blk_holder_ops *hops, struct file *bdev_file)
 {
 	struct bdev_handle *handle = kmalloc(sizeof(struct bdev_handle),
 					     GFP_KERNEL);
-	struct block_device *bdev;
 	bool unblock_events = true;
-	struct gendisk *disk;
+	struct gendisk *disk = bdev->bd_disk;
 	int ret;
 
+	handle = kmalloc(sizeof(struct bdev_handle), GFP_KERNEL);
 	if (!handle)
-		return ERR_PTR(-ENOMEM);
-
-	ret = devcgroup_check_permission(DEVCG_DEV_BLOCK,
-			MAJOR(dev), MINOR(dev),
-			((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
-			((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));
-	if (ret)
-		goto free_handle;
-
-	/* Blocking writes requires exclusive opener */
-	if (mode & BLK_OPEN_RESTRICT_WRITES && !holder) {
-		ret = -EINVAL;
-		goto free_handle;
-	}
-
-	bdev = blkdev_get_no_open(dev);
-	if (!bdev) {
-		ret = -ENXIO;
-		goto free_handle;
-	}
-	disk = bdev->bd_disk;
+		return -ENOMEM;
 
 	if (holder) {
 		mode |= BLK_OPEN_EXCL;
 		ret = bd_prepare_to_claim(bdev, holder, hops);
 		if (ret)
-			goto put_blkdev;
+			return ret;
 	} else {
-		if (WARN_ON_ONCE(mode & BLK_OPEN_EXCL)) {
-			ret = -EIO;
-			goto put_blkdev;
-		}
+		if (WARN_ON_ONCE(mode & BLK_OPEN_EXCL))
+			return -EIO;
 	}
 
 	disk_block_events(disk);
@@ -902,7 +897,22 @@  struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
 	handle->bdev = bdev;
 	handle->holder = holder;
 	handle->mode = mode;
-	return handle;
+
+	/*
+	 * Preserve backwards compatibility and allow large file access
+	 * even if userspace doesn't ask for it explicitly. Some mkfs
+	 * binary needs it. We might want to drop this workaround
+	 * during an unstable branch.
+	 */
+	bdev_file->f_flags |= O_LARGEFILE;
+	bdev_file->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
+	if (bdev_nowait(bdev))
+		bdev_file->f_mode |= FMODE_NOWAIT;
+	bdev_file->f_mapping = handle->bdev->bd_inode->i_mapping;
+	bdev_file->f_wb_err = filemap_sample_wb_err(bdev_file->f_mapping);
+	bdev_file->private_data = handle;
+
+	return 0;
 put_module:
 	module_put(disk->fops->owner);
 abort_claiming:
@@ -910,11 +920,8 @@  struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
 		bd_abort_claiming(bdev, holder);
 	mutex_unlock(&disk->open_mutex);
 	disk_unblock_events(disk);
-put_blkdev:
-	blkdev_put_no_open(bdev);
-free_handle:
 	kfree(handle);
-	return ERR_PTR(ret);
+	return ret;
 }
 
 static unsigned blk_to_file_flags(blk_mode_t mode)
@@ -954,29 +961,35 @@  struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
 				   const struct blk_holder_ops *hops)
 {
 	struct file *bdev_file;
-	struct bdev_handle *handle;
+	struct block_device *bdev;
 	unsigned int flags;
+	int ret;
 
-	handle = bdev_open_by_dev(dev, mode, holder, hops);
-	if (IS_ERR(handle))
-		return ERR_CAST(handle);
+	ret = bdev_permission(dev, 0, holder);
+	if (ret)
+		return ERR_PTR(ret);
+
+	bdev = blkdev_get_no_open(dev);
+	if (!bdev)
+		return ERR_PTR(-ENXIO);
 
 	flags = blk_to_file_flags(mode);
-	bdev_file = alloc_file_pseudo_noaccount(handle->bdev->bd_inode,
+	bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
 			blockdev_mnt, "", flags | O_LARGEFILE, &def_blk_fops);
 	if (IS_ERR(bdev_file)) {
-		bdev_release(handle);
+		blkdev_put_no_open(bdev);
 		return bdev_file;
 	}
-	ihold(handle->bdev->bd_inode);
+	bdev_file->f_mode &= ~FMODE_OPENED;
 
-	bdev_file->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
-	if (bdev_nowait(handle->bdev))
-		bdev_file->f_mode |= FMODE_NOWAIT;
-
-	bdev_file->f_mapping = handle->bdev->bd_inode->i_mapping;
-	bdev_file->f_wb_err = filemap_sample_wb_err(bdev_file->f_mapping);
-	bdev_file->private_data = handle;
+	ihold(bdev->bd_inode);
+	ret = bdev_open(bdev, mode, holder, hops, bdev_file);
+	if (ret) {
+		fput(bdev_file);
+		return ERR_PTR(ret);
+	}
+	/* Now that thing is opened. */
+	bdev_file->f_mode |= FMODE_OPENED;
 	return bdev_file;
 }
 EXPORT_SYMBOL(bdev_file_open_by_dev);
diff --git a/block/blk.h b/block/blk.h
index c9630774767d..19b15870284f 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -25,6 +25,12 @@  struct blk_flush_queue {
 	struct request		*flush_rq;
 };
 
+struct bdev_handle {
+	struct block_device *bdev;
+	void *holder;
+	blk_mode_t mode;
+};
+
 bool is_flush_rq(struct request *req);
 
 struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
@@ -517,7 +523,7 @@  static inline int req_ref_read(struct request *req)
 }
 
 void bdev_release(struct bdev_handle *handle);
-struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
-		const struct blk_holder_ops *hops);
-
+int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
+	      const struct blk_holder_ops *hops, struct file *bdev_file);
+int bdev_permission(dev_t dev, blk_mode_t mode, void *holder);
 #endif /* BLK_INTERNAL_H */
diff --git a/block/fops.c b/block/fops.c
index 0cf8cf72cdfa..81ff8c0ce32f 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -599,31 +599,25 @@  blk_mode_t file_to_blk_mode(struct file *file)
 
 static int blkdev_open(struct inode *inode, struct file *filp)
 {
-	struct bdev_handle *handle;
+	struct block_device *bdev;
 	blk_mode_t mode;
-
-	/*
-	 * Preserve backwards compatibility and allow large file access
-	 * even if userspace doesn't ask for it explicitly. Some mkfs
-	 * binary needs it. We might want to drop this workaround
-	 * during an unstable branch.
-	 */
-	filp->f_flags |= O_LARGEFILE;
-	filp->f_mode |= FMODE_BUF_RASYNC | FMODE_CAN_ODIRECT;
+	void *holder;
+	int ret;
 
 	mode = file_to_blk_mode(filp);
-	handle = bdev_open_by_dev(inode->i_rdev, mode,
-			mode & BLK_OPEN_EXCL ? filp : NULL, NULL);
-	if (IS_ERR(handle))
-		return PTR_ERR(handle);
+	holder = mode & BLK_OPEN_EXCL ? filp : NULL;
+	ret = bdev_permission(inode->i_rdev, mode, holder);
+	if (ret)
+		return ret;
 
-	if (bdev_nowait(handle->bdev))
-		filp->f_mode |= FMODE_NOWAIT;
+	bdev = blkdev_get_no_open(inode->i_rdev);
+	if (!bdev)
+		return -ENXIO;
 
-	filp->f_mapping = handle->bdev->bd_inode->i_mapping;
-	filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
-	filp->private_data = handle;
-	return 0;
+	ret = bdev_open(bdev, mode, holder, NULL, filp);
+	if (ret)
+		blkdev_put_no_open(bdev);
+	return ret;
 }
 
 static int blkdev_release(struct inode *inode, struct file *filp)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 495f55587207..2f5dbde23094 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1475,13 +1475,6 @@  extern const struct blk_holder_ops fs_holder_ops;
 	(BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES | \
 	 (((flags) & SB_RDONLY) ? 0 : BLK_OPEN_WRITE))
 
-/* @bdev_handle will be removed soon. */
-struct bdev_handle {
-	struct block_device *bdev;
-	void *holder;
-	blk_mode_t mode;
-};
-
 struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
 		const struct blk_holder_ops *hops);
 struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e9291e27cc47..6e0714d35d9b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1327,12 +1327,6 @@  struct super_block {
 	struct list_head	s_inodes_wb;	/* writeback inodes */
 } __randomize_layout;
 
-/* Temporary helper that will go away. */
-static inline struct bdev_handle *sb_bdev_handle(struct super_block *sb)
-{
-	return sb->s_bdev_file->private_data;
-}
-
 static inline struct user_namespace *i_user_ns(const struct inode *inode)
 {
 	return inode->i_sb->s_user_ns;