* Re: [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro()
2023-11-07 11:12 [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro() Yu Kuai
@ 2023-11-07 3:50 ` yebin (H)
2023-11-07 9:44 ` Yu Kuai
2023-11-07 15:15 ` Jens Axboe
2023-11-08 7:16 ` Christoph Hellwig
2 siblings, 1 reply; 6+ messages in thread
From: yebin (H) @ 2023-11-07 3:50 UTC (permalink / raw)
To: Yu Kuai, hch, axboe
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun
[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]
On 2023/11/7 19:12, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
>
> If one of the underlying disks of raid or dm is set to read-only, then
> each io will generate new log, which will cause message storm. This
> environment is indeed problematic, however we can't make sure our
> naive custormer won't do this, hence use pr_warn_ratelimited() to
> prevent message storm in this case.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> block/blk-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 9d51e9894ece..fdf25b8d6e78 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -501,8 +501,8 @@ static inline void bio_check_ro(struct bio *bio)
> if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) {
> if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
> return;
> - pr_warn("Trying to write to read-only block-device %pg\n",
> - bio->bi_bdev);
> + pr_warn_ratelimited("Trying to write to read-only block-device %pg\n",
> + bio->bi_bdev);
Acctually, before commit 57e95e4670d1 ("block: fix and cleanup
bio_check_ro") , there's only print warning once.
I wrote a patch earlier, set GD_ROWR_WARNED flag for disk after emit
warning. You can look at the patch in the
attachment, Is it possible to solve your problem.
> /* Older lvm-tools actually trigger this */
> }
> }
[-- Attachment #2: 0001-block-only-print-warning-once-when-write-to-readonly.patch --]
[-- Type: text/x-patch, Size: 2501 bytes --]
From 82b7dd41eb447e9fdd3d7c5d5e3a002a9f284d82 Mon Sep 17 00:00:00 2001
From: Ye Bin <yebin10@huawei.com>
Date: Wed, 9 Aug 2023 16:11:39 +0800
Subject: [PATCH] block: only print warning once when write to readonly bdev
After commit 57e95e4670d1 there will print warning unconditionally.As a result,
alarm logs are flooded.
To solve above issue, introduce GD_ROWR_WARNED state for every partition to
record if warning has been printed.
Fixes: 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
block/blk-core.c | 3 +++
block/genhd.c | 1 +
block/ioctl.c | 6 +++++-
include/linux/blkdev.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 9d51e9894ece..69d9757a013a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -501,6 +501,9 @@ static inline void bio_check_ro(struct bio *bio)
if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) {
if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
return;
+ if (test_and_set_bit(GD_ROWR_WARNED,
+ &bio->bi_bdev->bd_disk->state))
+ return;
pr_warn("Trying to write to read-only block-device %pg\n",
bio->bi_bdev);
/* Older lvm-tools actually trigger this */
diff --git a/block/genhd.c b/block/genhd.c
index c9d06f72c587..c05d2cd4a87b 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1455,6 +1455,7 @@ void set_disk_ro(struct gendisk *disk, bool read_only)
if (!test_and_clear_bit(GD_READ_ONLY, &disk->state))
return;
}
+ clear_bit(GD_ROWR_WARNED, &disk->state);
set_disk_ro_uevent(disk, read_only);
}
EXPORT_SYMBOL(set_disk_ro);
diff --git a/block/ioctl.c b/block/ioctl.c
index 4160f4e6bd5b..a2158c5e8e3e 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -394,7 +394,11 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd,
if (ret)
return ret;
}
- bdev->bd_read_only = n;
+ if (!bdev->bd_read_only != !n) {
+ bdev->bd_read_only = n;
+ clear_bit(GD_ROWR_WARNED, &bdev->bd_disk->state);
+ }
+
return 0;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 51fa7ffdee83..832f7f81e46c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -157,6 +157,7 @@ struct gendisk {
#define GD_ADDED 4
#define GD_SUPPRESS_PART_SCAN 5
#define GD_OWNS_QUEUE 6
+#define GD_ROWR_WARNED 7
struct mutex open_mutex; /* open/close mutex */
unsigned open_partitions; /* number of open partitions */
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro()
2023-11-07 11:12 [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro() Yu Kuai
2023-11-07 3:50 ` yebin (H)
@ 2023-11-07 15:15 ` Jens Axboe
2023-11-08 7:16 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2023-11-07 15:15 UTC (permalink / raw)
To: hch, Yu Kuai; +Cc: linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun
On Tue, 07 Nov 2023 19:12:47 +0800, Yu Kuai wrote:
> If one of the underlying disks of raid or dm is set to read-only, then
> each io will generate new log, which will cause message storm. This
> environment is indeed problematic, however we can't make sure our
> naive custormer won't do this, hence use pr_warn_ratelimited() to
> prevent message storm in this case.
>
>
> [...]
Applied, thanks!
[1/1] blk-core: use pr_warn_ratelimited() in bio_check_ro()
commit: 1b0a151c10a6d823f033023b9fdd9af72a89591b
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro()
2023-11-07 11:12 [PATCH] blk-core: use pr_warn_ratelimited() in bio_check_ro() Yu Kuai
2023-11-07 3:50 ` yebin (H)
2023-11-07 15:15 ` Jens Axboe
@ 2023-11-08 7:16 ` Christoph Hellwig
2023-11-08 8:17 ` Yu Kuai
2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2023-11-08 7:16 UTC (permalink / raw)
To: Yu Kuai; +Cc: hch, axboe, linux-block, linux-kernel, yukuai3, yi.zhang,
yangerkun
On Tue, Nov 07, 2023 at 07:12:47PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
>
> If one of the underlying disks of raid or dm is set to read-only, then
> each io will generate new log, which will cause message storm. This
> environment is indeed problematic, however we can't make sure our
> naive custormer won't do this, hence use pr_warn_ratelimited() to
> prevent message storm in this case.
Reducing the log spam sounds good, and I guess the single warning
would be even better.
That being said, why/how is the underlying device set to read-only?
If there is a good reason we should probably add a holder op to tell
the user about it so that it stop sending writes.
^ permalink raw reply [flat|nested] 6+ messages in thread