Re: [PATCH v2 30/34] bdev: remove bdev pointer from struct bdev_handle

Jan Kara
2024-02-01T10:57:24Z

On Tue 23-01-24 14:26:47, Christian Brauner wrote:
> We can always go directly via:
>
> * I_BDEV(bdev_file->f_inode)
> * I_BDEV(bdev_file->f_mapping->host)
>
> So keeping struct bdev in struct bdev_handle is redundant.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> block/bdev.c | 26 ++++++++++++--------------
> block/blk.h | 3 +--
> block/fops.c | 2 +-
> 3 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/block/bdev.c b/block/bdev.c
> index 34b9a16edb6e..71eaa1b5b7eb 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -51,8 +51,7 @@ EXPORT_SYMBOL(I_BDEV);
>
> struct block_device *file_bdev(struct file *bdev_file)
> {
> - struct bdev_handle *handle = bdev_file->private_data;
> - return handle->bdev;
> + return I_BDEV(bdev_file->f_mapping->host);
> }
> EXPORT_SYMBOL(file_bdev);
>
> @@ -894,7 +893,6 @@ int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
>
> if (unblock_events)
> disk_unblock_events(disk);
> - handle->bdev = bdev;
> handle->holder = holder;
> handle->mode = mode;
>
> @@ -908,7 +906,7 @@ int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
> 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_mapping = bdev->bd_inode->i_mapping;
> bdev_file->f_wb_err = filemap_sample_wb_err(bdev_file->f_mapping);
> bdev_file->private_data = handle;
>
> @@ -998,7 +996,7 @@ struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
> void *holder,
> const struct blk_holder_ops *hops)
> {
> - struct file *bdev_file;
> + struct file *file;
> dev_t dev;
> int error;
>
> @@ -1006,22 +1004,22 @@ struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
> if (error)
> return ERR_PTR(error);
>
> - bdev_file = bdev_file_open_by_dev(dev, mode, holder, hops);
> - if (!IS_ERR(bdev_file) && (mode & BLK_OPEN_WRITE)) {
> - struct bdev_handle *handle = bdev_file->private_data;
> - if (bdev_read_only(handle->bdev)) {
> - fput(bdev_file);
> - bdev_file = ERR_PTR(-EACCES);
> + file = bdev_file_open_by_dev(dev, mode, holder, hops);
> + if (!IS_ERR(file) && (mode & BLK_OPEN_WRITE)) {
> + if (bdev_read_only(file_bdev(file))) {
> + fput(file);
> + file = ERR_PTR(-EACCES);
> }
> }
>
> - return bdev_file;
> + return file;
> }
> EXPORT_SYMBOL(bdev_file_open_by_path);
>
> -void bdev_release(struct bdev_handle *handle)
> +void bdev_release(struct file *bdev_file)
> {
> - struct block_device *bdev = handle->bdev;
> + struct block_device *bdev = file_bdev(bdev_file);
> + struct bdev_handle *handle = bdev_file->private_data;
> struct gendisk *disk = bdev->bd_disk;
>
> /*
> diff --git a/block/blk.h b/block/blk.h
> index 19b15870284f..7ca24814f3a0 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -26,7 +26,6 @@ struct blk_flush_queue {
> };
>
> struct bdev_handle {
> - struct block_device *bdev;
> void *holder;
> blk_mode_t mode;
> };
> @@ -522,7 +521,7 @@ static inline int req_ref_read(struct request *req)
> return atomic_read(&req->ref);
> }
>
> -void bdev_release(struct bdev_handle *handle);
> +void bdev_release(struct file *bdev_file);
> 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);
> diff --git a/block/fops.c b/block/fops.c
> index 81ff8c0ce32f..5589bf9c3822 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -622,7 +622,7 @@ 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);
> + bdev_release(filp);
> return 0;
> }
>
>
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR