Linux-Block Archive mirror
 help / color / mirror / Atom feed
* regression on BLKRRPART ioctl for EIO
@ 2024-02-07  4:28 Saranya Muruganandam
  2024-02-12 15:44 ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Saranya Muruganandam @ 2024-02-07  4:28 UTC (permalink / raw
  To: linux-block, hch, Jens Axboe, sashal, Ming Lei

Hi,

I am noticing a regression on the BLKRRPART ioctl after we changed the
blkdev_reread_part() logic to reopen the device with commit
68e6582e8f2dc32fd2458b9926564faa1fb4560e["reopen the device in
blkdev_reread_part"]

We now ignore the errors that used to be returned for
bdev_disk_changed(). I see that this was explicitly fixed for -EBUSY
(commit 68e6582e8f2dc32fd2458b9926564faa1fb4560e: block: return -EBUSY
when there are open partitions in blkdev_reread_part)

The regression I am particularly interested in is when we get an -EIO
error from the disk and BLKRRPART incorrectly reports success when it
actually failed to reread partitions.

Looking for advice.

Thanks,
Saranya

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-02-07  4:28 regression on BLKRRPART ioctl for EIO Saranya Muruganandam
@ 2024-02-12 15:44 ` Christoph Hellwig
       [not found]   ` <CAP9s-Sr3_GVYBv-XObPRC9L27jJoQqX40d8g3gysEmy6VdQS1Q@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2024-02-12 15:44 UTC (permalink / raw
  To: Saranya Muruganandam; +Cc: linux-block, hch, Jens Axboe, sashal, Ming Lei

What scenario are you looking at?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
       [not found]   ` <CAP9s-Sr3_GVYBv-XObPRC9L27jJoQqX40d8g3gysEmy6VdQS1Q@mail.gmail.com>
@ 2024-02-13  4:01     ` Saranya Muruganandam
  2024-02-27  2:38       ` Saranya Muruganandam
  0 siblings, 1 reply; 10+ messages in thread
From: Saranya Muruganandam @ 2024-02-13  4:01 UTC (permalink / raw
  To: Christoph Hellwig; +Cc: linux-block, Jens Axboe, sashal, Ming Lei

When we fail to read from the disk, BLKRRPART used to be able to
capture and bubble this up to the caller.
It no longer does since we no longer capture the error from bdev_disk_changed.

Here is an example with fault-injection:

# echo 0 > /sys/kernel/debug/fail_make_request/interval
# echo 100 > /sys/kernel/debug/fail_make_request/probability
# echo -1 > /sys/kernel/debug/fail_make_request/times
# echo 1 > /sys/block/sdc/make-it-fail
# blockdev --rereadpt /dev/sdc # no error
# echo $?
0 # incorrectly reports success.

Whereas fdisk and sfdisk correctly report the issue :

# sfdisk /dev/sdc
sfdisk: cannot open /dev/sdc: Input/output error
# fdisk /dev/sdc

Welcome to fdisk (util-linux 2.28.2).
Changes will remain in memory only, until you decide to write them.
Be careful before using the write command.

fdisk: cannot open /dev/sdc: Input/output error

Best,
Saranya



On Mon, Feb 12, 2024 at 8:00 PM Saranya Muruganandam
<saranyamohan@google.com> wrote:
>
> When we fail to read from the disk, BLKRRPART used to be able to capture and bubble this up to the caller.
> It no longer does since we no longer capture the error from bdev_disk_changed.
>
> Here is an example with fault-injection:
>
> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> # echo -1 > /sys/kernel/debug/fail_make_request/times
> # echo 1 > /sys/block/sdc/make-it-fail
> # blockdev --rereadpt /dev/sdc # no error
> # echo $?
> 0 # incorrectly reports success.
>
> Whereas fdisk and sfdisk correctly report the issue :
>
> # sfdisk /dev/sdc
> sfdisk: cannot open /dev/sdc: Input/output error
> # fdisk /dev/sdc
>
> Welcome to fdisk (util-linux 2.28.2).
> Changes will remain in memory only, until you decide to write them.
> Be careful before using the write command.
>
> fdisk: cannot open /dev/sdc: Input/output error
>
> Best,
> Saranya
>
> On Mon, Feb 12, 2024 at 7:44 AM Christoph Hellwig <hch@lst.de> wrote:
>>
>> What scenario are you looking at?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-02-13  4:01     ` Saranya Muruganandam
@ 2024-02-27  2:38       ` Saranya Muruganandam
  2024-02-27  3:13         ` Yu Kuai
  0 siblings, 1 reply; 10+ messages in thread
From: Saranya Muruganandam @ 2024-02-27  2:38 UTC (permalink / raw
  To: Christoph Hellwig; +Cc: linux-block, Jens Axboe, sashal, Ming Lei

Hi,
Is there advice on how to fix this `blockdev --rereadpt` regression in
the kernel?
Would appreciate some advice.

Thanks,
Saranya


On Mon, Feb 12, 2024 at 8:01 PM Saranya Muruganandam
<saranyamohan@google.com> wrote:
>
> When we fail to read from the disk, BLKRRPART used to be able to
> capture and bubble this up to the caller.
> It no longer does since we no longer capture the error from bdev_disk_changed.
>
> Here is an example with fault-injection:
>
> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> # echo -1 > /sys/kernel/debug/fail_make_request/times
> # echo 1 > /sys/block/sdc/make-it-fail
> # blockdev --rereadpt /dev/sdc # no error
> # echo $?
> 0 # incorrectly reports success.
>
> Whereas fdisk and sfdisk correctly report the issue :
>
> # sfdisk /dev/sdc
> sfdisk: cannot open /dev/sdc: Input/output error
> # fdisk /dev/sdc
>
> Welcome to fdisk (util-linux 2.28.2).
> Changes will remain in memory only, until you decide to write them.
> Be careful before using the write command.
>
> fdisk: cannot open /dev/sdc: Input/output error
>
> Best,
> Saranya
>
>
>
> On Mon, Feb 12, 2024 at 8:00 PM Saranya Muruganandam
> <saranyamohan@google.com> wrote:
> >
> > When we fail to read from the disk, BLKRRPART used to be able to capture and bubble this up to the caller.
> > It no longer does since we no longer capture the error from bdev_disk_changed.
> >
> > Here is an example with fault-injection:
> >
> > # echo 0 > /sys/kernel/debug/fail_make_request/interval
> > # echo 100 > /sys/kernel/debug/fail_make_request/probability
> > # echo -1 > /sys/kernel/debug/fail_make_request/times
> > # echo 1 > /sys/block/sdc/make-it-fail
> > # blockdev --rereadpt /dev/sdc # no error
> > # echo $?
> > 0 # incorrectly reports success.
> >
> > Whereas fdisk and sfdisk correctly report the issue :
> >
> > # sfdisk /dev/sdc
> > sfdisk: cannot open /dev/sdc: Input/output error
> > # fdisk /dev/sdc
> >
> > Welcome to fdisk (util-linux 2.28.2).
> > Changes will remain in memory only, until you decide to write them.
> > Be careful before using the write command.
> >
> > fdisk: cannot open /dev/sdc: Input/output error
> >
> > Best,
> > Saranya
> >
> > On Mon, Feb 12, 2024 at 7:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >>
> >> What scenario are you looking at?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-02-27  2:38       ` Saranya Muruganandam
@ 2024-02-27  3:13         ` Yu Kuai
  2024-03-07 18:14           ` Saranya Muruganandam
  0 siblings, 1 reply; 10+ messages in thread
From: Yu Kuai @ 2024-02-27  3:13 UTC (permalink / raw
  To: Saranya Muruganandam, Christoph Hellwig
  Cc: linux-block, Jens Axboe, sashal, Ming Lei, yukuai (C)

Hi,

在 2024/02/27 10:38, Saranya Muruganandam 写道:
> Hi,
> Is there advice on how to fix this `blockdev --rereadpt` regression in
> the kernel?
> Would appreciate some advice.
> 
> Thanks,
> Saranya
> 
> 
> On Mon, Feb 12, 2024 at 8:01 PM Saranya Muruganandam
> <saranyamohan@google.com> wrote:
>>
>> When we fail to read from the disk, BLKRRPART used to be able to
>> capture and bubble this up to the caller.
>> It no longer does since we no longer capture the error from bdev_disk_changed.
>>
>> Here is an example with fault-injection:
>>
>> # echo 0 > /sys/kernel/debug/fail_make_request/interval
>> # echo 100 > /sys/kernel/debug/fail_make_request/probability
>> # echo -1 > /sys/kernel/debug/fail_make_request/times
>> # echo 1 > /sys/block/sdc/make-it-fail
>> # blockdev --rereadpt /dev/sdc # no error
>> # echo $?
>> 0 # incorrectly reports success.

I take a look at this, and it's right the root cause is commit
4601b4b130de ("block: reopen the device in blkdev_reread_part") ignore
errors from bdev_disk_changed() now.

I think we can fix this by returning error from bdev_get_whole() if
bdev_disk_changed() failed, this will cause that open disk to fail now,
however, I think this can be acceptable.

Christoph, do you think so? Or we should distinguish ioctl and open
device and only let ioctl to fail.

Thanks,
Kuai


>>
>> Whereas fdisk and sfdisk correctly report the issue :
>>
>> # sfdisk /dev/sdc
>> sfdisk: cannot open /dev/sdc: Input/output error
>> # fdisk /dev/sdc
>>
>> Welcome to fdisk (util-linux 2.28.2).
>> Changes will remain in memory only, until you decide to write them.
>> Be careful before using the write command.
>>
>> fdisk: cannot open /dev/sdc: Input/output error
>>
>> Best,
>> Saranya
>>
>>
>>
>> On Mon, Feb 12, 2024 at 8:00 PM Saranya Muruganandam
>> <saranyamohan@google.com> wrote:
>>>
>>> When we fail to read from the disk, BLKRRPART used to be able to capture and bubble this up to the caller.
>>> It no longer does since we no longer capture the error from bdev_disk_changed.
>>>
>>> Here is an example with fault-injection:
>>>
>>> # echo 0 > /sys/kernel/debug/fail_make_request/interval
>>> # echo 100 > /sys/kernel/debug/fail_make_request/probability
>>> # echo -1 > /sys/kernel/debug/fail_make_request/times
>>> # echo 1 > /sys/block/sdc/make-it-fail
>>> # blockdev --rereadpt /dev/sdc # no error
>>> # echo $?
>>> 0 # incorrectly reports success.
>>>
>>> Whereas fdisk and sfdisk correctly report the issue :
>>>
>>> # sfdisk /dev/sdc
>>> sfdisk: cannot open /dev/sdc: Input/output error
>>> # fdisk /dev/sdc
>>>
>>> Welcome to fdisk (util-linux 2.28.2).
>>> Changes will remain in memory only, until you decide to write them.
>>> Be careful before using the write command.
>>>
>>> fdisk: cannot open /dev/sdc: Input/output error
>>>
>>> Best,
>>> Saranya
>>>
>>> On Mon, Feb 12, 2024 at 7:44 AM Christoph Hellwig <hch@lst.de> wrote:
>>>>
>>>> What scenario are you looking at?
> 
> .
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-02-27  3:13         ` Yu Kuai
@ 2024-03-07 18:14           ` Saranya Muruganandam
  2024-03-08 16:32             ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Saranya Muruganandam @ 2024-03-07 18:14 UTC (permalink / raw
  To: Yu Kuai
  Cc: Christoph Hellwig, linux-block, Jens Axboe, sashal, Ming Lei,
	yukuai (C)

> I think we can fix this by returning error from bdev_get_whole() if
> bdev_disk_changed() failed, this will cause that open disk to fail now,
> however, I think this can be acceptable.

Thanks for the response!
I agree this would fix the regression for the ioctl.
However, since returning an error from blkdev_get_whole is new
behavior, I wasn't sure what all parts it affects.

So this is just a ping to let you know that I am also waiting to hear
from Christoph.


On Mon, Feb 26, 2024 at 7:13 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/02/27 10:38, Saranya Muruganandam 写道:
> > Hi,
> > Is there advice on how to fix this `blockdev --rereadpt` regression in
> > the kernel?
> > Would appreciate some advice.
> >
> > Thanks,
> > Saranya
> >
> >
> > On Mon, Feb 12, 2024 at 8:01 PM Saranya Muruganandam
> > <saranyamohan@google.com> wrote:
> >>
> >> When we fail to read from the disk, BLKRRPART used to be able to
> >> capture and bubble this up to the caller.
> >> It no longer does since we no longer capture the error from bdev_disk_changed.
> >>
> >> Here is an example with fault-injection:
> >>
> >> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> >> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> >> # echo -1 > /sys/kernel/debug/fail_make_request/times
> >> # echo 1 > /sys/block/sdc/make-it-fail
> >> # blockdev --rereadpt /dev/sdc # no error
> >> # echo $?
> >> 0 # incorrectly reports success.
>
> I take a look at this, and it's right the root cause is commit
> 4601b4b130de ("block: reopen the device in blkdev_reread_part") ignore
> errors from bdev_disk_changed() now.
>
> I think we can fix this by returning error from bdev_get_whole() if
> bdev_disk_changed() failed, this will cause that open disk to fail now,
> however, I think this can be acceptable.
>
> Christoph, do you think so? Or we should distinguish ioctl and open
> device and only let ioctl to fail.
>
> Thanks,
> Kuai
>
>
> >>
> >> Whereas fdisk and sfdisk correctly report the issue :
> >>
> >> # sfdisk /dev/sdc
> >> sfdisk: cannot open /dev/sdc: Input/output error
> >> # fdisk /dev/sdc
> >>
> >> Welcome to fdisk (util-linux 2.28.2).
> >> Changes will remain in memory only, until you decide to write them.
> >> Be careful before using the write command.
> >>
> >> fdisk: cannot open /dev/sdc: Input/output error
> >>
> >> Best,
> >> Saranya
> >>
> >>
> >>
> >> On Mon, Feb 12, 2024 at 8:00 PM Saranya Muruganandam
> >> <saranyamohan@google.com> wrote:
> >>>
> >>> When we fail to read from the disk, BLKRRPART used to be able to capture and bubble this up to the caller.
> >>> It no longer does since we no longer capture the error from bdev_disk_changed.
> >>>
> >>> Here is an example with fault-injection:
> >>>
> >>> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> >>> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> >>> # echo -1 > /sys/kernel/debug/fail_make_request/times
> >>> # echo 1 > /sys/block/sdc/make-it-fail
> >>> # blockdev --rereadpt /dev/sdc # no error
> >>> # echo $?
> >>> 0 # incorrectly reports success.
> >>>
> >>> Whereas fdisk and sfdisk correctly report the issue :
> >>>
> >>> # sfdisk /dev/sdc
> >>> sfdisk: cannot open /dev/sdc: Input/output error
> >>> # fdisk /dev/sdc
> >>>
> >>> Welcome to fdisk (util-linux 2.28.2).
> >>> Changes will remain in memory only, until you decide to write them.
> >>> Be careful before using the write command.
> >>>
> >>> fdisk: cannot open /dev/sdc: Input/output error
> >>>
> >>> Best,
> >>> Saranya
> >>>
> >>> On Mon, Feb 12, 2024 at 7:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >>>>
> >>>> What scenario are you looking at?
> >
> > .
> >
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-03-07 18:14           ` Saranya Muruganandam
@ 2024-03-08 16:32             ` Christoph Hellwig
  2024-03-20  0:20               ` Saranya Muruganandam
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2024-03-08 16:32 UTC (permalink / raw
  To: Saranya Muruganandam
  Cc: Yu Kuai, Christoph Hellwig, linux-block, Jens Axboe, sashal,
	Ming Lei, yukuai (C)

On Thu, Mar 07, 2024 at 10:14:05AM -0800, Saranya Muruganandam wrote:
> > I think we can fix this by returning error from bdev_get_whole() if
> > bdev_disk_changed() failed, this will cause that open disk to fail now,
> > however, I think this can be acceptable.
> 
> Thanks for the response!
> I agree this would fix the regression for the ioctl.
> However, since returning an error from blkdev_get_whole is new
> behavior, I wasn't sure what all parts it affects.
> 
> So this is just a ping to let you know that I am also waiting to hear
> from Christoph.

I'd hate returning a failure and changing the interface, but I haven't
come up with anything better yet.  Let me thing about this a bit more.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-03-08 16:32             ` Christoph Hellwig
@ 2024-03-20  0:20               ` Saranya Muruganandam
  2024-03-20  1:51                 ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Saranya Muruganandam @ 2024-03-20  0:20 UTC (permalink / raw
  To: Christoph Hellwig
  Cc: Yu Kuai, linux-block, Jens Axboe, sashal, Ming Lei, yukuai (C)

Any advice yet?

I really appreciate the help.

thanks,
Saranya

On Fri, Mar 8, 2024 at 8:32 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Mar 07, 2024 at 10:14:05AM -0800, Saranya Muruganandam wrote:
> > > I think we can fix this by returning error from bdev_get_whole() if
> > > bdev_disk_changed() failed, this will cause that open disk to fail now,
> > > however, I think this can be acceptable.
> >
> > Thanks for the response!
> > I agree this would fix the regression for the ioctl.
> > However, since returning an error from blkdev_get_whole is new
> > behavior, I wasn't sure what all parts it affects.
> >
> > So this is just a ping to let you know that I am also waiting to hear
> > from Christoph.
>
> I'd hate returning a failure and changing the interface, but I haven't
> come up with anything better yet.  Let me thing about this a bit more.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-03-20  0:20               ` Saranya Muruganandam
@ 2024-03-20  1:51                 ` Christoph Hellwig
  2024-04-05  2:04                   ` Saranya Muruganandam
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2024-03-20  1:51 UTC (permalink / raw
  To: Saranya Muruganandam
  Cc: Christoph Hellwig, Yu Kuai, linux-block, Jens Axboe, sashal,
	Ming Lei, yukuai (C)

Please try the patch below:

I would also relly help to have a blktsts test case to show your
issue and verify that it is fixed.

diff --git a/block/bdev.c b/block/bdev.c
index e7adaaf1c21927..51071d371863e0 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -645,6 +645,14 @@ static void blkdev_flush_mapping(struct block_device *bdev)
 	bdev_write_inode(bdev);
 }
 
+static void blkdev_put_whole(struct block_device *bdev)
+{
+	if (atomic_dec_and_test(&bdev->bd_openers))
+		blkdev_flush_mapping(bdev);
+	if (bdev->bd_disk->fops->release)
+		bdev->bd_disk->fops->release(bdev->bd_disk);
+}
+
 static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode)
 {
 	struct gendisk *disk = bdev->bd_disk;
@@ -663,20 +671,21 @@ static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode)
 
 	if (!atomic_read(&bdev->bd_openers))
 		set_init_blocksize(bdev);
-	if (test_bit(GD_NEED_PART_SCAN, &disk->state))
-		bdev_disk_changed(disk, false);
 	atomic_inc(&bdev->bd_openers);
+	if (test_bit(GD_NEED_PART_SCAN, &disk->state)) {
+		/*
+		 * Only return scanning errors if we are called from conexts
+		 * that explicitly want them, e.g. the BLKRRPART ioctl.
+		 */
+		ret = bdev_disk_changed(disk, false);
+		if (ret && (mode & BLK_OPEN_STRICT_SCAN)) {
+			blkdev_put_whole(bdev);
+			return ret;
+		}
+	}
 	return 0;
 }
 
-static void blkdev_put_whole(struct block_device *bdev)
-{
-	if (atomic_dec_and_test(&bdev->bd_openers))
-		blkdev_flush_mapping(bdev);
-	if (bdev->bd_disk->fops->release)
-		bdev->bd_disk->fops->release(bdev->bd_disk);
-}
-
 static int blkdev_get_part(struct block_device *part, blk_mode_t mode)
 {
 	struct gendisk *disk = part->bd_disk;
diff --git a/block/ioctl.c b/block/ioctl.c
index 0c76137adcaaa5..128f503828cee7 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -562,7 +562,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode,
 			return -EACCES;
 		if (bdev_is_partition(bdev))
 			return -EINVAL;
-		return disk_scan_partitions(bdev->bd_disk, mode);
+		return disk_scan_partitions(bdev->bd_disk,
+				mode | BLK_OPEN_STRICT_SCAN);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
 	case BLKTRACETEARDOWN:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f9b87c39cab047..272ce42f297cfe 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -128,6 +128,8 @@ typedef unsigned int __bitwise blk_mode_t;
 #define BLK_OPEN_WRITE_IOCTL	((__force blk_mode_t)(1 << 4))
 /* open is exclusive wrt all other BLK_OPEN_WRITE opens to the device */
 #define BLK_OPEN_RESTRICT_WRITES	((__force blk_mode_t)(1 << 5))
+/* return partition scanning errors */
+#define BLK_OPEN_STRICT_SCAN	((__force blk_mode_t)(1 << 5))
 
 struct gendisk {
 	/*

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: regression on BLKRRPART ioctl for EIO
  2024-03-20  1:51                 ` Christoph Hellwig
@ 2024-04-05  2:04                   ` Saranya Muruganandam
  0 siblings, 0 replies; 10+ messages in thread
From: Saranya Muruganandam @ 2024-04-05  2:04 UTC (permalink / raw
  To: Christoph Hellwig
  Cc: Yu Kuai, linux-block, Jens Axboe, sashal, Ming Lei, yukuai (C)

Hi Christoph,
Apologies for the delay.

Thanks a ton for your patch! Your patch solution works for me.
I sent https://lore.kernel.org/all/20240405014253.748627-1-saranyamohan@google.com/
for the block layer patch
and https://lore.kernel.org/all/20240405015657.751659-1-saranyamohan@google.com/
for the blktest added.

Once I get confirmation that they look good, I have the changes for
the older LTS which I can share.
This patch doesn't cleanly apply but your idea of flagging and using
that to return errors, fixes the regression
for stable kernels like 5.10.

Thanks again.

-Saranya


On Tue, Mar 19, 2024 at 6:51 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Please try the patch below:
>
> I would also relly help to have a blktsts test case to show your
> issue and verify that it is fixed.
>
> diff --git a/block/bdev.c b/block/bdev.c
> index e7adaaf1c21927..51071d371863e0 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -645,6 +645,14 @@ static void blkdev_flush_mapping(struct block_device *bdev)
>         bdev_write_inode(bdev);
>  }
>
> +static void blkdev_put_whole(struct block_device *bdev)
> +{
> +       if (atomic_dec_and_test(&bdev->bd_openers))
> +               blkdev_flush_mapping(bdev);
> +       if (bdev->bd_disk->fops->release)
> +               bdev->bd_disk->fops->release(bdev->bd_disk);
> +}
> +
>  static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode)
>  {
>         struct gendisk *disk = bdev->bd_disk;
> @@ -663,20 +671,21 @@ static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode)
>
>         if (!atomic_read(&bdev->bd_openers))
>                 set_init_blocksize(bdev);
> -       if (test_bit(GD_NEED_PART_SCAN, &disk->state))
> -               bdev_disk_changed(disk, false);
>         atomic_inc(&bdev->bd_openers);
> +       if (test_bit(GD_NEED_PART_SCAN, &disk->state)) {
> +               /*
> +                * Only return scanning errors if we are called from conexts
> +                * that explicitly want them, e.g. the BLKRRPART ioctl.
> +                */
> +               ret = bdev_disk_changed(disk, false);
> +               if (ret && (mode & BLK_OPEN_STRICT_SCAN)) {
> +                       blkdev_put_whole(bdev);
> +                       return ret;
> +               }
> +       }
>         return 0;
>  }
>
> -static void blkdev_put_whole(struct block_device *bdev)
> -{
> -       if (atomic_dec_and_test(&bdev->bd_openers))
> -               blkdev_flush_mapping(bdev);
> -       if (bdev->bd_disk->fops->release)
> -               bdev->bd_disk->fops->release(bdev->bd_disk);
> -}
> -
>  static int blkdev_get_part(struct block_device *part, blk_mode_t mode)
>  {
>         struct gendisk *disk = part->bd_disk;
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 0c76137adcaaa5..128f503828cee7 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -562,7 +562,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode,
>                         return -EACCES;
>                 if (bdev_is_partition(bdev))
>                         return -EINVAL;
> -               return disk_scan_partitions(bdev->bd_disk, mode);
> +               return disk_scan_partitions(bdev->bd_disk,
> +                               mode | BLK_OPEN_STRICT_SCAN);
>         case BLKTRACESTART:
>         case BLKTRACESTOP:
>         case BLKTRACETEARDOWN:
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index f9b87c39cab047..272ce42f297cfe 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -128,6 +128,8 @@ typedef unsigned int __bitwise blk_mode_t;
>  #define BLK_OPEN_WRITE_IOCTL   ((__force blk_mode_t)(1 << 4))
>  /* open is exclusive wrt all other BLK_OPEN_WRITE opens to the device */
>  #define BLK_OPEN_RESTRICT_WRITES       ((__force blk_mode_t)(1 << 5))
> +/* return partition scanning errors */
> +#define BLK_OPEN_STRICT_SCAN   ((__force blk_mode_t)(1 << 5))
>
>  struct gendisk {
>         /*

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-04-05  2:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-07  4:28 regression on BLKRRPART ioctl for EIO Saranya Muruganandam
2024-02-12 15:44 ` Christoph Hellwig
     [not found]   ` <CAP9s-Sr3_GVYBv-XObPRC9L27jJoQqX40d8g3gysEmy6VdQS1Q@mail.gmail.com>
2024-02-13  4:01     ` Saranya Muruganandam
2024-02-27  2:38       ` Saranya Muruganandam
2024-02-27  3:13         ` Yu Kuai
2024-03-07 18:14           ` Saranya Muruganandam
2024-03-08 16:32             ` Christoph Hellwig
2024-03-20  0:20               ` Saranya Muruganandam
2024-03-20  1:51                 ` Christoph Hellwig
2024-04-05  2:04                   ` Saranya Muruganandam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).