On Thu, Feb 01, 2024 at 12:20:08PM +0100, Jan Kara wrote:
> 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.
Fixed.
