Re: [PATCH v2 32/34] block: remove bdev_handle completely

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue 23-01-24 14:26:49, Christian Brauner wrote:
> We just need to use the holder to indicate whether a block device open
> was exclusive or not. We did use to do that before but had to give that
> up once we switched to struct bdev_handle. Before struct bdev_handle we
> only stashed stuff in file->private_data if this was an exclusive open
> but after struct bdev_handle we always set file->private_data to a
> struct bdev_handle and so we had to use bdev_handle->mode or
> bdev_handle->holder. Now that we don't use struct bdev_handle anymore we
> can revert back to the old behavior.
> 
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>

Two small comments below.

> diff --git a/block/fops.c b/block/fops.c
> index f56bdfe459de..a0bff2c0d88d 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -569,7 +569,6 @@ static int blkdev_fsync(struct file *filp, loff_t start, loff_t end,
>  blk_mode_t file_to_blk_mode(struct file *file)
>  {
>  	blk_mode_t mode = 0;
> -	struct bdev_handle *handle = file->private_data;
>  
>  	if (file->f_mode & FMODE_READ)
>  		mode |= BLK_OPEN_READ;
> @@ -579,8 +578,8 @@ blk_mode_t file_to_blk_mode(struct file *file)
>  	 * do_dentry_open() clears O_EXCL from f_flags, use handle->mode to
>  	 * determine whether the open was exclusive for already open files.
>  	 */
^^^ This comment needs update now...

> -	if (handle)
> -		mode |= handle->mode & BLK_OPEN_EXCL;
> +	if (file->private_data)
> +		mode |= BLK_OPEN_EXCL;
>  	else if (file->f_flags & O_EXCL)
>  		mode |= BLK_OPEN_EXCL;
>  	if (file->f_flags & O_NDELAY)
> @@ -601,12 +600,17 @@ static int blkdev_open(struct inode *inode, struct file *filp)
>  {
>  	struct block_device *bdev;
>  	blk_mode_t mode;
> -	void *holder;
>  	int ret;
>  
> +	/*
> +	 * Use the file private data to store the holder for exclusive opens.
> +	 * file_to_blk_mode relies on it being present to set BLK_OPEN_EXCL.
> +	 */
> +	if (filp->f_flags & O_EXCL)
> +		filp->private_data = filp;

Well, if we have O_EXCL in f_flags here, then file_to_blk_mode() on the
next line is going to do the right thing and set BLK_OPEN_EXCL even
without filp->private_data. So this shound't be needed AFAICT.

>  	mode = file_to_blk_mode(filp);
> -	holder = mode & BLK_OPEN_EXCL ? filp : NULL;
> -	ret = bdev_permission(inode->i_rdev, mode, holder);
> +	ret = bdev_permission(inode->i_rdev, mode, filp->private_data);
>  	if (ret)
>  		return ret;

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux