| Message ID | 20231107111247.2157820-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-core: use pr_warn_ratelimited() in bio_check_ro() | expand |
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 */ > } > }
Hi, 在 2023/11/07 11:50, yebin (H) 写道: > > > 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. Yes, this can work, other than that I think the flag should be in block_device instead of gendisk. However, I'm not sure which is better for now, hope Christoph or Ming or anyone can give some advise. Thanks, Kuai >> /* Older lvm-tools actually trigger this */ >> } >> } >
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,
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.
Hi, 在 2023/11/08 15:16, Christoph Hellwig 写道: > 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. Got it. Jens, I see that you already apply this version, do you want me to send a new version to generate single warning for each block_device? > > 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. > Our custormer use blockdev --getro to underlying device directly, they're trying to forbid other users to write underlying device, so in this case I think set underlying device to read-only is not okay because it's already write opened, however I'm not sure if we want the ioctl to fail. Thanks, Kuai > > . >
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); /* Older lvm-tools actually trigger this */ } }