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

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

 



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.




[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