block: ratelimit warning in bio_check_ro

Commit Message

Ladislav Michl June 7, 2023, 6:28 a.m. UTC

From: Ladislav Michl <[e-mail]>

Until 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
a WARN_ONCE was used to print a warning. Current pr_warn causes
log flood, so use pr_warn_ratelimited instead.
Once there adjust message to match the one used in bio_check_eod.

Signed-off-by: Ladislav Michl <[e-mail]>
Fixes: 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
---
 block/blk-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig June 7, 2023, 6:30 a.m. UTC | #1

On Wed, Jun 07, 2023 at 08:28:22AM +0200, Ladislav Michl wrote:
> From: Ladislav Michl <[e-mail]>
> 
> Until 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
> a WARN_ONCE was used to print a warning. Current pr_warn causes
> log flood, so use pr_warn_ratelimited instead.
> Once there adjust message to match the one used in bio_check_eod.

Do you have a case that hits this?  Beause we'd really need to fix it.

Otherwise this looks ok to me.

Ladislav Michl June 7, 2023, 7:45 a.m. UTC | #2

Hi Christoph,

On Wed, Jun 07, 2023 at 08:30:02AM +0200, Christoph Hellwig wrote:
> On Wed, Jun 07, 2023 at 08:28:22AM +0200, Ladislav Michl wrote:
> > From: Ladislav Michl <[e-mail]>
> > 
> > Until 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
> > a WARN_ONCE was used to print a warning. Current pr_warn causes
> > log flood, so use pr_warn_ratelimited instead.
> > Once there adjust message to match the one used in bio_check_eod.
> 
> Do you have a case that hits this?  Beause we'd really need to fix it.

I hit that while working on customer's embedded board. There's eMMC
where boot partitions are locked after upgrade by writing 1 into
force_ro sysfs file. Pending writes are triggering this warnign.
Of course update scripts was fixed meanwhile and knowing what
process triggered warning was quite helpful. So there's nothing
to fix, it is just improved diagnostic and returning to (almost)
old behaviour.

> Otherwise this looks ok to me.

Ladislav Michl Aug. 14, 2023, 7:56 p.m. UTC | #3

On Wed, Jun 07, 2023 at 09:45:35AM +0200, Ladislav Michl wrote:
> Hi Christoph,
> 
> On Wed, Jun 07, 2023 at 08:30:02AM +0200, Christoph Hellwig wrote:
> > On Wed, Jun 07, 2023 at 08:28:22AM +0200, Ladislav Michl wrote:
> > > From: Ladislav Michl <[e-mail]>
> > > 
> > > Until 57e95e4670d1 ("block: fix and cleanup bio_check_ro")
> > > a WARN_ONCE was used to print a warning. Current pr_warn causes
> > > log flood, so use pr_warn_ratelimited instead.
> > > Once there adjust message to match the one used in bio_check_eod.
> > 
> > Do you have a case that hits this?  Beause we'd really need to fix it.
> 
> I hit that while working on customer's embedded board. There's eMMC
> where boot partitions are locked after upgrade by writing 1 into
> force_ro sysfs file. Pending writes are triggering this warnign.
> Of course update scripts was fixed meanwhile and knowing what
> process triggered warning was quite helpful. So there's nothing
> to fix, it is just improved diagnostic and returning to (almost)
> old behaviour.
> 
> > Otherwise this looks ok to me.

Any more thoughts on that? I do not push on it being mainlined, just sorting
patch stack for one of custom boards...

Thanks,
	l.

Patch

diff --git a/block/blk-core.c b/block/blk-core.c
index 1da77e7d6289..fbd9f4102703 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -496,8 +496,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("%s: attempt to write to read-only device %pg\n",
+				    current->comm, bio->bi_bdev);
 		/* Older lvm-tools actually trigger this */
 	}
 }