[02/29] block: Use bdev_open_by_dev() in blkdev_open()

Message ID	20230811110504.27514-2-jack@suse.cz (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-fsdevel-owner@vger.kernel.org> From: Jan Kara <jack@suse.cz> To: <linux-fsdevel@vger.kernel.org> Cc: <linux-block@vger.kernel.org>, Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz> Subject: [PATCH 02/29] block: Use bdev_open_by_dev() in blkdev_open() Date: Fri, 11 Aug 2023 13:04:33 +0200 Message-Id: <20230811110504.27514-2-jack@suse.cz> In-Reply-To: <20230810171429.31759-1-jack@suse.cz> References: <20230810171429.31759-1-jack@suse.cz> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	block: Make blkdev_get_by_() return handle \| expand [v2,0/29] block: Make blkdev_get_by_() return handle [01/29] block: Provide bdev_open_* functions [02/29] block: Use bdev_open_by_dev() in blkdev_open() [03/29] block: Use bdev_open_by_dev() in disk_scan_partitions() and blkdev_bszset() [04/29] drdb: Convert to use bdev_open_by_path() [05/29] pktcdvd: Convert to bdev_open_by_dev() [06/29] rnbd-srv: Convert to use bdev_open_by_path() [07/29] xen/blkback: Convert to bdev_open_by_dev() [08/29] zram: Convert to use bdev_open_by_dev() [09/29] bcache: Convert to bdev_open_by_path() [10/29] dm: Convert to bdev_open_by_dev() [11/29] md: Convert to bdev_open_by_dev() [12/29] mtd: block2mtd: Convert to bdev_open_by_dev/path() [13/29] nvmet: Convert to bdev_open_by_path() [14/29] s390/dasd: Convert to bdev_open_by_path() [15/29] scsi: target: Convert to bdev_open_by_path() [16/29] PM: hibernate: Convert to bdev_open_by_dev() [17/29] PM: hibernate: Drop unused snapshot_test argument [18/29] mm/swap: Convert to use bdev_open_by_dev() [19/29] fs: Convert to bdev_open_by_dev() [20/29] btrfs: Convert to bdev_open_by_path() [21/29] erofs: Convert to use bdev_open_by_path() [22/29] ext4: Convert to bdev_open_by_dev() [23/29] f2fs: Convert to bdev_open_by_dev/path() [24/29] jfs: Convert to bdev_open_by_dev() [25/29] nfs/blocklayout: Convert to use bdev_open_by_dev/path() [26/29] ocfs2: Convert to use bdev_open_by_dev() [27/29] reiserfs: Convert to bdev_open_by_dev/path() [28/29] xfs: Convert to bdev_open_by_path() [29/29] block: Remove blkdev_get_by_*() functions

Message ID

20230811110504.27514-2-jack@suse.cz (mailing list archive)

State

New, archived

Headers

From: Jan Kara <jack@suse.cz>
To: <linux-fsdevel@vger.kernel.org>
Cc: <linux-block@vger.kernel.org>,
        Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz>
Subject: [PATCH 02/29] block: Use bdev_open_by_dev() in blkdev_open()
Date: Fri, 11 Aug 2023 13:04:33 +0200
Message-Id: <20230811110504.27514-2-jack@suse.cz>
In-Reply-To: <20230810171429.31759-1-jack@suse.cz>
References: <20230810171429.31759-1-jack@suse.cz>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

block: Make blkdev_get_by_*() return handle | expand

Commit Message

Jan Kara Aug. 11, 2023, 11:04 a.m. UTC

Convert blkdev_open() to use bdev_open_by_dev(). To be able to propagate
handle from blkdev_open() to blkdev_release() we need to stop using
existence of file->private_data to determine exclusive block device
opens. Use bdev_handle->mode for this purpose since file->f_flags
isn't usable for this (O_EXCL is cleared from the flags during open).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bdev.c           |  3 +++
 block/blk.h            |  1 -
 block/fops.c           | 33 +++++++++++++++------------------
 block/ioctl.c          |  4 ++--
 include/linux/blkdev.h |  1 +
 5 files changed, 21 insertions(+), 21 deletions(-)

Comments

Christoph Hellwig Aug. 11, 2023, 12:25 p.m. UTC | #1

On Fri, Aug 11, 2023 at 01:04:33PM +0200, Jan Kara wrote:
> +	blk_mode_t open_mode = ((struct bdev_handle *)file->private_data)->mode;

Nit: but I find it much more readable to just have a local bdev_handle
variable vs these deep references including casts.  This also appears
in a few others places.

Jan Kara Aug. 14, 2023, 1:51 p.m. UTC | #2

On Fri 11-08-23 05:25:41, Christoph Hellwig wrote:
> On Fri, Aug 11, 2023 at 01:04:33PM +0200, Jan Kara wrote:
> > +	blk_mode_t open_mode = ((struct bdev_handle *)file->private_data)->mode;
> 
> Nit: but I find it much more readable to just have a local bdev_handle
> variable vs these deep references including casts.  This also appears
> in a few others places.

OK, I've added the local variable to reduce typecasting and the depth of
dereferences.

								Honza

Al Viro Aug. 25, 2023, 1:14 a.m. UTC | #3

On Fri, Aug 11, 2023 at 01:04:33PM +0200, Jan Kara wrote:

> @@ -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;


> index 3be11941fb2d..47f216d8697f 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -575,7 +575,7 @@ long blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  {
>  	struct block_device *bdev = I_BDEV(file->f_mapping->host);
>  	void __user *argp = (void __user *)arg;
> -	blk_mode_t mode = file_to_blk_mode(file);
> +	blk_mode_t mode = ((struct bdev_handle *)file->private_data)->mode;

Take a look at sd_ioctl() and note that fcntl(2) can be used to set/clear O_NDELAY.
The current variant works since we recalculate mode every time; this one will end up
stuck with whatever we had at open time.  Note that Christoph's series could do this
in blkdev_ioctl()
-       /*
-        * O_NDELAY can be altered using fcntl(.., F_SETFL, ..), so we have
-        * to updated it before every ioctl.
-        */
-       if (file->f_flags & O_NDELAY)
-               mode |= FMODE_NDELAY;
-       else
-               mode &= ~FMODE_NDELAY;
precisely because his file_to_blk_mode() picks O_NDELAY from flags when blkdev_ioctl()
calls it.

The same goes for compat counterpart of that thing.  Both need to deal with that
scenario - you need something that would pick the O_NDELAY updates from ->f_flags.

Al, trying to catch up on the awful pile of mail that has accumulated over the 3 months
of being net.dead...

diff --git a/block/bdev.c b/block/bdev.c
index 74fc2aeaab2c..32c0ef5abad4 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -861,6 +861,9 @@  struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
 	}
 	handle->bdev = bdev;
 	handle->holder = holder;
+	if (holder)
+		mode |= BLK_OPEN_EXCL;
+	handle->mode = mode;
 	return handle;
 }
 EXPORT_SYMBOL(bdev_open_by_dev);
diff --git a/block/blk.h b/block/blk.h
index 608c5dcc516b..43b80dc78918 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -464,7 +464,6 @@  extern struct device_attribute dev_attr_events_poll_msecs;
 
 extern struct attribute_group blk_trace_attr_group;
 
-blk_mode_t file_to_blk_mode(struct file *file);
 int truncate_bdev_range(struct block_device *bdev, blk_mode_t mode,
 		loff_t lstart, loff_t lend);
 long blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg);
diff --git a/block/fops.c b/block/fops.c
index a286bf3325c5..6a3bc2b7d4f3 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -470,7 +470,7 @@  static int blkdev_fsync(struct file *filp, loff_t start, loff_t end,
 	return error;
 }
 
-blk_mode_t file_to_blk_mode(struct file *file)
+static blk_mode_t file_to_blk_mode(struct file *file)
 {
 	blk_mode_t mode = 0;
 
@@ -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;
@@ -496,7 +496,8 @@  blk_mode_t file_to_blk_mode(struct file *file)
 
 static int blkdev_open(struct inode *inode, struct file *filp)
 {
-	struct block_device *bdev;
+	struct bdev_handle *handle;
+	blk_mode_t mode;
 
 	/*
 	 * Preserve backwards compatibility and allow large file access
@@ -507,29 +508,24 @@  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);
-	if (IS_ERR(bdev))
-		return PTR_ERR(bdev);
+	mode = file_to_blk_mode(filp);
+	handle = bdev_open_by_dev(inode->i_rdev, mode,
+			mode & BLK_OPEN_EXCL ? filp : NULL, NULL);
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
 
-	if (bdev_nowait(bdev))
+	if (bdev_nowait(handle->bdev))
 		filp->f_mode |= FMODE_NOWAIT;
 
-	filp->f_mapping = bdev->bd_inode->i_mapping;
+	filp->f_mapping = handle->bdev->bd_inode->i_mapping;
 	filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
+	filp->private_data = handle;
 	return 0;
 }
 
 static int blkdev_release(struct inode *inode, struct file *filp)
 {
-	blkdev_put(I_BDEV(filp->f_mapping->host), filp->private_data);
+	bdev_release(filp->private_data);
 	return 0;
 }
 
@@ -630,6 +626,7 @@  static long blkdev_fallocate(struct file *file, int mode, loff_t start,
 {
 	struct inode *inode = bdev_file_inode(file);
 	struct block_device *bdev = I_BDEV(inode);
+	blk_mode_t open_mode = ((struct bdev_handle *)file->private_data)->mode;
 	loff_t end = start + len - 1;
 	loff_t isize;
 	int error;
@@ -659,7 +656,7 @@  static long blkdev_fallocate(struct file *file, int mode, loff_t start,
 	filemap_invalidate_lock(inode->i_mapping);
 
 	/* Invalidate the page cache, including dirty pages. */
-	error = truncate_bdev_range(bdev, file_to_blk_mode(file), start, end);
+	error = truncate_bdev_range(bdev, open_mode, start, end);
 	if (error)
 		goto fail;
 
diff --git a/block/ioctl.c b/block/ioctl.c
index 3be11941fb2d..47f216d8697f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -575,7 +575,7 @@  long blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 {
 	struct block_device *bdev = I_BDEV(file->f_mapping->host);
 	void __user *argp = (void __user *)arg;
-	blk_mode_t mode = file_to_blk_mode(file);
+	blk_mode_t mode = ((struct bdev_handle *)file->private_data)->mode;
 	int ret;
 
 	switch (cmd) {
@@ -636,7 +636,7 @@  long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 	void __user *argp = compat_ptr(arg);
 	struct block_device *bdev = I_BDEV(file->f_mapping->host);
 	struct gendisk *disk = bdev->bd_disk;
-	blk_mode_t mode = file_to_blk_mode(file);
+	blk_mode_t mode = ((struct bdev_handle *)file->private_data)->mode;
 
 	switch (cmd) {
 	/* These need separate implementations for the data structure */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8bdaf89fd879..0e89ad5c645c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1476,6 +1476,7 @@  extern const struct blk_holder_ops fs_holder_ops;
 struct bdev_handle {
 	struct block_device *bdev;
 	void *holder;
+	blk_mode_t mode;
 };
 
 struct block_device *blkdev_get_by_dev(dev_t dev, blk_mode_t mode, void *holder,

[02/29] block: Use bdev_open_by_dev() in blkdev_open()

Commit Message

Comments

Patch