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

[Jan Kara <jack@suse.cz>]

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