Re: [PATCH v2 31/34] block: use file->f_op to indicate restricted writes

Jan Kara
2024-02-01T11:09:04Z

On Tue 23-01-24 14:26:48, Christian Brauner wrote:
> Make it possible to detected a block device that was opened with
> restricted write access solely based on its file operations that it was
> opened with. This avoids wasting an FMODE_* flag.
>
> def_blk_fops isn't needed to check whether something is a block device
> checking the inode type is enough for that. And def_blk_fops_restricted
> can be kept private to the block layer.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>

I don't think we need def_blk_fops_restricted. If we have BLK_OPEN_WRITE
file against a bdev with bdev_writes_blocked() == true, we are sure this is
the handle blocking other writes so we can unblock them in
bdev_yield_write_access()...

Honza

> ---
> block/bdev.c | 16 ++++++++++++----
> block/blk.h | 2 ++
> block/fops.c | 3 +++
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/block/bdev.c b/block/bdev.c
> index 71eaa1b5b7eb..9d96a43f198d 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -799,13 +799,16 @@ static void bdev_claim_write_access(struct block_device *bdev, blk_mode_t mode)
> bdev->bd_writers++;
> }
>
> -static void bdev_yield_write_access(struct block_device *bdev, blk_mode_t mode)
> +static void bdev_yield_write_access(struct file *bdev_file, blk_mode_t mode)
> {
> + struct block_device *bdev;
> +
> if (bdev_allow_write_mounted)
> return;
>
> + bdev = file_bdev(bdev_file);
> /* Yield exclusive or shared write access. */
> - if (mode & BLK_OPEN_RESTRICT_WRITES)
> + if (bdev_file->f_op == &def_blk_fops_restricted)
> bdev_unblock_writes(bdev);
> else if (mode & BLK_OPEN_WRITE)
> bdev->bd_writers--;
> @@ -959,6 +962,7 @@ 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;
> + const struct file_operations *blk_fops;
> struct block_device *bdev;
> unsigned int flags;
> int ret;
> @@ -972,8 +976,12 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> return ERR_PTR(-ENXIO);
>
> flags = blk_to_file_flags(mode);
> + if (mode & BLK_OPEN_RESTRICT_WRITES)
> + blk_fops = &def_blk_fops_restricted;
> + else
> + blk_fops = &def_blk_fops;
> bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
> - blockdev_mnt, "", flags | O_LARGEFILE, &def_blk_fops);
> + blockdev_mnt, "", flags | O_LARGEFILE, blk_fops);
> if (IS_ERR(bdev_file)) {
> blkdev_put_no_open(bdev);
> return bdev_file;
> @@ -1033,7 +1041,7 @@ void bdev_release(struct file *bdev_file)
> sync_blockdev(bdev);
>
> mutex_lock(&disk->open_mutex);
> - bdev_yield_write_access(bdev, handle->mode);
> + bdev_yield_write_access(bdev_file, handle->mode);
>
> if (handle->holder)
> bd_end_claim(bdev, handle->holder);
> diff --git a/block/blk.h b/block/blk.h
> index 7ca24814f3a0..dfa958909c54 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -9,6 +9,8 @@
>
> struct elevator_type;
>
> +extern const struct file_operations def_blk_fops_restricted;
> +
> /* Max future timer expiry for timeouts */
> #define BLK_MAX_TIMEOUT (5 * HZ)
>
> diff --git a/block/fops.c b/block/fops.c
> index 5589bf9c3822..f56bdfe459de 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -862,6 +862,9 @@ const struct file_operations def_blk_fops = {
> .fallocate = blkdev_fallocate,
> };
>
> +/* Indicator that this block device is opened with restricted write access. */
> +const struct file_operations def_blk_fops_restricted = def_blk_fops;
> +
> static __init int blkdev_init(void)
> {
> return bioset_init(&blkdev_dio_pool, 4,
>
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR