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
