Skip to contentSkip to conversation

limit error rate

PATCH In review Bernd Schubert 3 participants 5 messages Apr 12, 2008 – Apr 28, 2008 MD RAID

lore
Share & tools
lore

Bernd Schubert

author
#0 lore raw

PATCH 1/1 limit error rate

Hello,

last night we had scsi problems and a hardware raid unit was offlined during heavy i/o. While this happened we got for about 3 minutes a huge number messages like these

Apr 12 03:36:07 pfs1n14 kernel: [197510.696595] raid5:md7: read error not correctable (sector 2993096568 on sdj2).

I guess the high error rate is responsible for not scheduling other events - during this time the system was not pingable and in the end also other devices run into scsi command timeouts causing problems on these unrelated devices as well.

Signed-off-by Bernd Schubert <[email protected]>
drivers/md/raid5.c +9 -6
index b162b83..3c82dfb 100644
@@ -1141,10 +1141,12 @@ static void raid5_end_read_request(struct bio * bi, int error)
                set_bit(R5_UPTODATE, &sh->dev[i].flags);
                if (test_bit(R5_ReadError, &sh->dev[i].flags)) {
                        rdev = conf->disks[i].rdev;
-                       printk(KERN_INFO "raid5:%s: read error corrected (%lu sectors at %llu on %s)\n",
-                              mdname(conf->mddev), STRIPE_SECTORS,
-                              (unsigned long long)(sh->sector + rdev->data_offset),
-                              bdevname(rdev->bdev, b));
+                       if (printk_ratelimit())
+                               printk(KERN_INFO "raid5:%s: read error corrected"
+                                      " (%lu sectors at %llu on %s)\n",
+                                      mdname(conf->mddev), STRIPE_SECTORS,
+                                      (unsigned long long)(sh->sector + rdev->data_offset),
+                                      bdevname(rdev->bdev, b));
                        clear_bit(R5_ReadError, &sh->dev[i].flags);
                        clear_bit(R5_ReWrite, &sh->dev[i].flags);
                }

@@ -1157,12 +1159,13 @@ static void raid5_end_read_request(struct bio * bi, int error)
 
                clear_bit(R5_UPTODATE, &sh->dev[i].flags);
                atomic_inc(&rdev->read_errors);
-               if (conf->mddev->degraded)
+               if (conf->mddev->degraded && printk_ratelimit())
                        printk(KERN_WARNING "raid5:%s: read error not correctable (sector %llu on %s).\n",
                               mdname(conf->mddev),
                               (unsigned long long)(sh->sector + rdev->data_offset),
                               bdn);
-               else if (test_bit(R5_ReWrite, &sh->dev[i].flags))
+               else if (test_bit(R5_ReWrite, &sh->dev[i].flags) && 
+                        printk_ratelimit())
                        /* Oh, no!!! */
                        printk(KERN_WARNING "raid5:%s: read error NOT corrected!! (sector %llu on %s).\n",
                               mdname(conf->mddev),
#1 lore raw
Quoted 8 lines · Bernd Schubert

Hello,

last night we had scsi problems and a hardware raid unit was offlined during heavy i/o. While this happened we got for about 3 minutes a huge number messages like these

 Apr 12 03:36:07 pfs1n14 kernel: [197510.696595] raid5:md7: read error not correctable (sector 2993096568 on sdj2).
Quoted 8 lines

I guess the high error rate is responsible for not scheduling other events - during this time the system was not pingable and in the end also other devices run into scsi command timeouts causing problems on these unrelated devices as well.

Signed-off-by Bernd Schubert <[email protected]>

Hi Bernd,

This patch is whitespace damaged (tabs-->spaces). Can you resend as an attachment?

Thanks,
Dan

author
#2 lore raw

Hello Dan,

Quoted 1 line · Dan Williams

On Sat, Apr 12, 2008 at 11:16 AM, Bernd Schubert <[email protected]>

Quoted 15 lines · wrote:
Quoted 15 lines

Hello,

last night we had scsi problems and a hardware raid unit was offlined during heavy i/o. While this happened we got for about 3 minutes a huge number messages like these

 Apr 12 03:36:07 pfs1n14 kernel: [197510.696595] raid5:md7: read error

not correctable (sector 2993096568 on sdj2).

I guess the high error rate is responsible for not scheduling other events - during this time the system was not pingable and in the end also other devices run into scsi command timeouts causing problems on these unrelated devices as well.

Quoted 6 lines
Quoted 1 line
Signed-off-by Bernd Schubert <[email protected]>

Hi Bernd,

This patch is whitespace damaged (tabs-->spaces). Can you resend as an attachment?

hmm, don't know how I managed to do that. Probably copied it from the shell... I have attached it this time. I also just added another printk_ratelimit().

Btw, from my point of view the

if (printk_ratelimit())

	printk("print output");

looks odd. I just don't see why the API isn't

printk_ratelimit("print output");

Oh well, modifying this all over the code would give a huge almost useless patch _only_ improving the beauty of code.

Thanks,
Bernd

drivers/md/raid5.c +10 -7
index b162b83..60d3442 100644
@@ -1141,10 +1141,12 @@ static void raid5_end_read_request(struct bio * bi, int error)
 		set_bit(R5_UPTODATE, &sh->dev[i].flags);
 		if (test_bit(R5_ReadError, &sh->dev[i].flags)) {
 			rdev = conf->disks[i].rdev;
-			printk(KERN_INFO "raid5:%s: read error corrected (%lu sectors at %llu on %s)\n",
-			       mdname(conf->mddev), STRIPE_SECTORS,
-			       (unsigned long long)(sh->sector + rdev->data_offset),
-			       bdevname(rdev->bdev, b));
+			if (printk_ratelimit())
+				printk(KERN_INFO "raid5:%s: read error corrected"
+				       " (%lu sectors at %llu on %s)\n",
+				       mdname(conf->mddev), STRIPE_SECTORS,
+				       (unsigned long long)(sh->sector + rdev->data_offset),
+				       bdevname(rdev->bdev, b));
 			clear_bit(R5_ReadError, &sh->dev[i].flags);
 			clear_bit(R5_ReWrite, &sh->dev[i].flags);
 		}

@@ -1157,19 +1159,20 @@ static void raid5_end_read_request(struct bio * bi, int error)
 
 		clear_bit(R5_UPTODATE, &sh->dev[i].flags);
 		atomic_inc(&rdev->read_errors);
-		if (conf->mddev->degraded)
+		if (conf->mddev->degraded && printk_ratelimit())
 			printk(KERN_WARNING "raid5:%s: read error not correctable (sector %llu on %s).\n",
 			       mdname(conf->mddev),
 			       (unsigned long long)(sh->sector + rdev->data_offset),
 			       bdn);
-		else if (test_bit(R5_ReWrite, &sh->dev[i].flags))
+		else if (test_bit(R5_ReWrite, &sh->dev[i].flags) && 
+			 printk_ratelimit())
 			/* Oh, no!!! */
 			printk(KERN_WARNING "raid5:%s: read error NOT corrected!! (sector %llu on %s).\n",
 			       mdname(conf->mddev),
 			       (unsigned long long)(sh->sector + rdev->data_offset),
 			       bdn);
 		else if (atomic_read(&rdev->read_errors)
-			 > conf->max_nr_stripes)
+			 > conf->max_nr_stripes && printk_ratelimit())
 			printk(KERN_WARNING
 			       "raid5:%s: Too many read errors, failing device %s.\n",
 			       mdname(conf->mddev), bdn);
#3 lore raw
Quoted 15 lines · On Thursday April 24, [email protected] wrote:
drivers/md/raid5.c +3 -2
@@ -1157,19 +1159,20 @@ static void raid5_end_read_request(struct bio * bi, int error)
 
 		clear_bit(R5_UPTODATE, &sh->dev[i].flags);
 		atomic_inc(&rdev->read_errors);
-		if (conf->mddev->degraded)
+		if (conf->mddev->degraded && printk_ratelimit())
 			printk(KERN_WARNING "raid5:%s: read error not correctable (sector %llu on %s).\n",
 			       mdname(conf->mddev),
 			       (unsigned long long)(sh->sector + rdev->data_offset),
 			       bdn);
-		else if (test_bit(R5_ReWrite, &sh->dev[i].flags))
+		else if (test_bit(R5_ReWrite, &sh->dev[i].flags) && 
+			 printk_ratelimit())
 			/* Oh, no!!! */

drivers/md/raid5.c static void raid5_end_read_request(struct bio * bi, int error) · line 1178

Quoted 10 lines
drivers/md/raid5.c +1 -1
 			printk(KERN_WARNING "raid5:%s: read error NOT corrected!! (sector %llu on %s).\n",
 			       mdname(conf->mddev),
 			       (unsigned long long)(sh->sector + rdev->data_offset),
 			       bdn);
 		else if (atomic_read(&rdev->read_errors)
-			 > conf->max_nr_stripes)
+			 > conf->max_nr_stripes && printk_ratelimit())
 			printk(KERN_WARNING
 			       "raid5:%s: Too many read errors, failing device %s.\n",
 			       mdname(conf->mddev), bdn);

This is not good, you are subtly changing semantics. You have changed:

if (A)

      X;
  else if (B)
      Y;
  else if (C)
      Z;
  else
      W;

to
if (A && foo())

      X;
  else if (B && foo())
      Y;
  else if (C && foo())
      Z;
  else
      W;

If 'A' and not 'foo()', you want nothing to happen, but you will actually get one of Y, Z, or W happening, all of which are wrong.

Quoted 9 lines

Btw, from my point of view the

if (printk_ratelimit())

	printk("print output");

looks odd. I just don't see why the API isn't

printk_ratelimit("print output");

Very sensible. I've put

  #define printk_rl  printk_ratelimit() ?: printk

at the start of raid5.c, and used it to fix the problematic printks.

Thanks,
NeilBrown

author
#4 lore raw
Quoted 22 lines · Neil Brown

This is not good, you are subtly changing semantics. You have changed:

if (A)

      X;
  else if (B)
      Y;
  else if (C)
      Z;
  else
      W;

to
if (A && foo())

      X;
  else if (B && foo())
      Y;
  else if (C && foo())
      Z;
  else
      W;
Quoted 2 lines

If 'A' and not 'foo()', you want nothing to happen, but you will actually get one of Y, Z, or W happening, all of which are wrong.

Ouch, right. I shouldn't write patches in the middle of the night...

Quoted 8 lines
Quoted 8 lines

Btw, from my point of view the

if (printk_ratelimit())

	printk("print output");

looks odd. I just don't see why the API isn't

Quoted 7 lines
Quoted 1 line

printk_ratelimit("print output");

Very sensible. I've put

  #define printk_rl  printk_ratelimit() ?: printk

at the start of raid5.c, and used it to fix the problematic printks.

Great!

Thanks a lot,
Bernd