[PATCH] block: reopen the device in blkdev_reread_part

[PATCH] block: reopen the device in blkdev_reread_part

[Christoph Hellwig]

Historically the BLKRRPART ioctls called into the now defunct ->revalidate
method, which caused the sd driver to check if any media is present.
When the ->revalidate method was removed this revalidation was lost,
leading to lots of I/O errors when using the eject command.  Fix this by
reopening the device to rescan the partitions, and thus calling the
revalidation logic in the sd driver.

Fixes: 471bd0af544b ("sd: use bdev_check_media_change")
Reported--by: Tom Seewald <tseewald@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Tom Seewald <tseewald@gmail.com>
---
 block/ioctl.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index d61d652078f41c..ff241e663c018f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -81,20 +81,27 @@ static int compat_blkpg_ioctl(struct block_device *bdev,
 }
 #endif
 
-static int blkdev_reread_part(struct block_device *bdev)
+static int blkdev_reread_part(struct block_device *bdev, fmode_t mode)
 {
-	int ret;
+	struct block_device *tmp;
 
 	if (!disk_part_scan_enabled(bdev->bd_disk) || bdev_is_partition(bdev))
 		return -EINVAL;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	mutex_lock(&bdev->bd_mutex);
-	ret = bdev_disk_changed(bdev, false);
-	mutex_unlock(&bdev->bd_mutex);
+	/*
+	 * Reopen the device to revalidate the driver state and force a
+	 * partition rescan.
+	 */
+	mode &= ~FMODE_EXCL;
+	set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
 
-	return ret;
+	tmp = blkdev_get_by_dev(bdev->bd_dev, mode, NULL);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+	blkdev_put(tmp, mode);
+	return 0;
 }
 
 static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
@@ -498,7 +505,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 		bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
 		return 0;
 	case BLKRRPART:
-		return blkdev_reread_part(bdev);
+		return blkdev_reread_part(bdev, mode);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
 	case BLKTRACETEARDOWN:
-- 
2.29.2

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Ming Lei]

On Tue, Feb 23, 2021 at 04:18:22PM +0100, Christoph Hellwig wrote:
> Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> method, which caused the sd driver to check if any media is present.
> When the ->revalidate method was removed this revalidation was lost,
> leading to lots of I/O errors when using the eject command.  Fix this by
> reopening the device to rescan the partitions, and thus calling the
> revalidation logic in the sd driver.
> 
> Fixes: 471bd0af544b ("sd: use bdev_check_media_change")
> Reported--by: Tom Seewald <tseewald@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Tom Seewald <tseewald@gmail.com>
> ---
>  block/ioctl.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index d61d652078f41c..ff241e663c018f 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -81,20 +81,27 @@ static int compat_blkpg_ioctl(struct block_device *bdev,
>  }
>  #endif
>  
> -static int blkdev_reread_part(struct block_device *bdev)
> +static int blkdev_reread_part(struct block_device *bdev, fmode_t mode)
>  {
> -	int ret;
> +	struct block_device *tmp;
>  
>  	if (!disk_part_scan_enabled(bdev->bd_disk) || bdev_is_partition(bdev))
>  		return -EINVAL;
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EACCES;
>  
> -	mutex_lock(&bdev->bd_mutex);
> -	ret = bdev_disk_changed(bdev, false);
> -	mutex_unlock(&bdev->bd_mutex);
> +	/*
> +	 * Reopen the device to revalidate the driver state and force a
> +	 * partition rescan.
> +	 */
> +	mode &= ~FMODE_EXCL;
> +	set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>  
> -	return ret;
> +	tmp = blkdev_get_by_dev(bdev->bd_dev, mode, NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +	blkdev_put(tmp, mode);
> +	return 0;
>  }
>  
>  static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
> @@ -498,7 +505,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
>  		bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
>  		return 0;
>  	case BLKRRPART:
> -		return blkdev_reread_part(bdev);
> +		return blkdev_reread_part(bdev, mode);
>  	case BLKTRACESTART:
>  	case BLKTRACESTOP:
>  	case BLKTRACETEARDOWN:
> -- 
> 2.29.2
> 

Looks fine,

Reviewed-by: Ming Lei <ming.lei@redhat.com>


-- 
Ming

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Minwoo Im]

On 21-02-23 16:18:22, Christoph Hellwig wrote:
> Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> method, which caused the sd driver to check if any media is present.
> When the ->revalidate method was removed this revalidation was lost,
> leading to lots of I/O errors when using the eject command.  Fix this by
> reopening the device to rescan the partitions, and thus calling the
> revalidation logic in the sd driver.

It looks like a related issue that I've reported in [1].  And this looks
much better!

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>

[1] https://lore.kernel.org/linux-block/20210126002901.5533-2-minwoo.im.dev@gmail.com/

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Jens Axboe]

On 2/23/21 8:18 AM, Christoph Hellwig wrote:
> Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> method, which caused the sd driver to check if any media is present.
> When the ->revalidate method was removed this revalidation was lost,
> leading to lots of I/O errors when using the eject command.  Fix this by
> reopening the device to rescan the partitions, and thus calling the
> revalidation logic in the sd driver.

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Christoph Hellwig]

On Wed, Feb 24, 2021 at 10:52:02AM +0900, Minwoo Im wrote:
> On 21-02-23 16:18:22, Christoph Hellwig wrote:
> > Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> > method, which caused the sd driver to check if any media is present.
> > When the ->revalidate method was removed this revalidation was lost,
> > leading to lots of I/O errors when using the eject command.  Fix this by
> > reopening the device to rescan the partitions, and thus calling the
> > revalidation logic in the sd driver.
> 
> It looks like a related issue that I've reported in [1].  And this looks
> much better!

I don't think it fixes the block size issue, does it?

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Minwoo Im]

On 21-02-24 08:26:03, Christoph Hellwig wrote:
> On Wed, Feb 24, 2021 at 10:52:02AM +0900, Minwoo Im wrote:
> > On 21-02-23 16:18:22, Christoph Hellwig wrote:
> > > Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> > > method, which caused the sd driver to check if any media is present.
> > > When the ->revalidate method was removed this revalidation was lost,
> > > leading to lots of I/O errors when using the eject command.  Fix this by
> > > reopening the device to rescan the partitions, and thus calling the
> > > revalidation logic in the sd driver.
> > 
> > It looks like a related issue that I've reported in [1].  And this looks
> > much better!
> 
> I don't think it fixes the block size issue, does it?

Uh... Sorry for the noise.  This reopen is not the first shot so that the
block size will not be updated because bd_openers is not 0.
set_init_blocksize is not being invoked.

Sorry please ignore this noise.

Re: [PATCH] block: reopen the device in blkdev_reread_part

[Minwoo Im]

On 21-02-24 17:32:46, Minwoo Im wrote:
> On 21-02-24 08:26:03, Christoph Hellwig wrote:
> > On Wed, Feb 24, 2021 at 10:52:02AM +0900, Minwoo Im wrote:
> > > On 21-02-23 16:18:22, Christoph Hellwig wrote:
> > > > Historically the BLKRRPART ioctls called into the now defunct ->revalidate
> > > > method, which caused the sd driver to check if any media is present.
> > > > When the ->revalidate method was removed this revalidation was lost,
> > > > leading to lots of I/O errors when using the eject command.  Fix this by
> > > > reopening the device to rescan the partitions, and thus calling the
> > > > revalidation logic in the sd driver.
> > > 
> > > It looks like a related issue that I've reported in [1].  And this looks
> > > much better!
> > 
> > I don't think it fixes the block size issue, does it?
> 
> Uh... Sorry for the noise.  This reopen is not the first shot so that the
> block size will not be updated because bd_openers is not 0.
> set_init_blocksize is not being invoked.
> 
> Sorry please ignore this noise.

FYI,

the block size issue has no issue based on the current tree with
8dc932d3e8af ("Revert "block: simplify set_init_blocksize" to regain
lost performance")