| Message ID | 20230704122224.16257-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Make blkdev_get_by_*() return handle | expand |
On Tue, Jul 04, 2023 at 02:21:29PM +0200, Jan Kara wrote: > Use file->f_flags instead of file->private_data for determining whether > we should set BLK_OPEN_EXCL flag. This allows us to remove somewhat > awkward setting of file->private_data before calling file_to_blk_mode() > and it also makes following conversion to blkdev_get_handle_by_dev() > simpler. While this looks a lot nicer, I don't think it actually works given that do_dentry_open clears out O_EXCL, and thus it won't be set when calling into blkdev_ioctl.
On Thu 06-07-23 08:35:23, Christoph Hellwig wrote: > On Tue, Jul 04, 2023 at 02:21:29PM +0200, Jan Kara wrote: > > Use file->f_flags instead of file->private_data for determining whether > > we should set BLK_OPEN_EXCL flag. This allows us to remove somewhat > > awkward setting of file->private_data before calling file_to_blk_mode() > > and it also makes following conversion to blkdev_get_handle_by_dev() > > simpler. > > While this looks a lot nicer, I don't think it actually works given that > do_dentry_open clears out O_EXCL, and thus it won't be set when > calling into blkdev_ioctl. Aha, good point! So I need to workaround this a bit differently. I think the best would be to have file_to_blk_mode() for blkdev_open() only and make the other places (which already have bdev_handle available from struct file) use bdev_handle->mode. But I have to come up with a sane transition to that state :). Honza
On Thu, Jul 06, 2023 at 06:35:56PM +0200, Jan Kara wrote: > > While this looks a lot nicer, I don't think it actually works given that > > do_dentry_open clears out O_EXCL, and thus it won't be set when > > calling into blkdev_ioctl. > > Aha, good point! So I need to workaround this a bit differently. I think > the best would be to have file_to_blk_mode() for blkdev_open() only and > make the other places (which already have bdev_handle available from > struct file) use bdev_handle->mode. Exactly. > But I have to come up with a sane > transition to that state :). I think you can simply do it in the patch switching the block device to be handle based.
diff --git a/block/fops.c b/block/fops.c index a286bf3325c5..b6aa470c09ae 100644 --- a/block/fops.c +++ b/block/fops.c @@ -478,7 +478,7 @@ blk_mode_t file_to_blk_mode(struct file *file) mode |= BLK_OPEN_READ; if (file->f_mode & FMODE_WRITE) mode |= BLK_OPEN_WRITE; - if (file->private_data) + if (file->f_flags & O_EXCL) mode |= BLK_OPEN_EXCL; if (file->f_flags & O_NDELAY) mode |= BLK_OPEN_NDELAY; @@ -497,6 +497,7 @@ blk_mode_t file_to_blk_mode(struct file *file) static int blkdev_open(struct inode *inode, struct file *filp) { struct block_device *bdev; + blk_mode_t mode; /* * Preserve backwards compatibility and allow large file access @@ -507,18 +508,14 @@ static int blkdev_open(struct inode *inode, struct file *filp) filp->f_flags |= O_LARGEFILE; filp->f_mode |= FMODE_BUF_RASYNC; - /* - * Use the file private data to store the holder for exclusive openes. - * file_to_blk_mode relies on it being present to set BLK_OPEN_EXCL. - */ - if (filp->f_flags & O_EXCL) - filp->private_data = filp; - - bdev = blkdev_get_by_dev(inode->i_rdev, file_to_blk_mode(filp), - filp->private_data, NULL); + mode = file_to_blk_mode(filp); + bdev = blkdev_get_by_dev(inode->i_rdev, mode, + mode & BLK_OPEN_EXCL ? filp : NULL, NULL); if (IS_ERR(bdev)) return PTR_ERR(bdev); + if (mode & BLK_OPEN_EXCL) + filp->private_data = filp; if (bdev_nowait(bdev)) filp->f_mode |= FMODE_NOWAIT;
Use file->f_flags instead of file->private_data for determining whether we should set BLK_OPEN_EXCL flag. This allows us to remove somewhat awkward setting of file->private_data before calling file_to_blk_mode() and it also makes following conversion to blkdev_get_handle_by_dev() simpler. Signed-off-by: Jan Kara <jack@suse.cz> --- block/fops.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)