Fix some spurious errors at bootup

PATCH In review Andy Lutomirski 3 participants 10 messages Mar 8, 2014 – Mar 11, 2014

Subsystems (4)

SCSI Char and misc drivers Megaraid SCSI/SAS drivers Intel management engine (mei)

lore

Share & tools

lore

Andy Lutomirski

author

2014-03-08 18:58 UTC #0 lore raw

cover Fix some spurious errors at bootup

A bunch of drivers that my system loads spew out KERN_ERR or severity-less messages at startup. These message are not errors. Fix them.

Andy Lutomirski (4):
mei: Don't log an error when the MEI device is quirked off
megaraid: Downgrade success messages from KERN_ERR to dev_dbg
sr: Add a missing KERN_INFO
raid6: Add severity levels to raid6 initialization messages

 drivers/misc/mei/pci-me.c                   |  2 +-
 drivers/scsi/megaraid/megaraid_sas_base.c   |  2 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c |  2 +-
 drivers/scsi/sr.c                           |  3 ++-
 lib/raid6/algos.c                           | 13 +++++++------
 5 files changed, 12 insertions(+), 10 deletions(-)

Signature · 1 line

1.8.5.3

Andy Lutomirski

author

2014-03-08 18:58 UTC #1 lore raw

PATCH 1/4 mei: Don't log an error when the MEI device is quirked off

If an administrator wants to use MEI, they can search the logs for 'mei'. Otherwise they don't need a glaring reminder that their hardware doesn't actually support MEI.

Signed-off-by Andy Lutomirski <[email protected]>

---
 drivers/misc/mei/pci-me.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/misc/mei/pci-me.c +1 -1

index ddadd08..809021e 100644
@@ -127,7 +127,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if (!mei_me_quirk_probe(pdev, ent)) {
 		err = -ENODEV;
-		goto end;
+		return err;  /* No point in logging an error. */
 	}
 
 	/* enable pci dev */

Signature · 1 line

1.8.5.3

Winkler, Tomas

2014-03-11 10:03 UTC #2 lore raw

RE: [PATCH 1/4] mei: Don't log an error when the MEI device is quirked off

Quoted 23 lines

-----Original Message-----
From: Andy Lutomirski [mailto:[email protected]]
Sent: Saturday, March 08, 2014 20:58
To: [email protected]
Cc: [email protected]; Andy Lutomirski; Winkler, Tomas
Subject: [PATCH 1/4] mei: Don't log an error when the MEI device is quirked off
If an administrator wants to use MEI, they can search the logs for 'mei'. Otherwise they don't need a glaring reminder that their hardware doesn't actually support MEI.
Signed-off-by Andy Lutomirski <[email protected]>
---
 drivers/misc/mei/pci-me.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/misc/mei/pci-me.c +0 -0
index ddadd08..809021e 100644

drivers/misc/mei/pci-me.c static int mei_me_probe(struct pci_dev *pdev, const · line 132

Quoted 7 lines

drivers/misc/mei/pci-me.c +1 -1

@@ -127,7 +127,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
 struct pci_device_id *ent)
 
 	if (!mei_me_quirk_probe(pdev, ent)) {
 		err = -ENODEV;
-		goto end;
+		return err;  /* No point in logging an error. */
 	}

I would like to have at least some info/debug message logged,
the issue was discovered on specific DELL machines and there is a little worry that on some systems it can be false positive.

Thanks
Tomas

Andy Lutomirski

author

2014-03-11 22:18 UTC #3 lore raw

Re: [PATCH 1/4] mei: Don't log an error when the MEI device is quirked off

Quoted 25 lines · Tomas

Quoted 25 lines
-----Original Message-----
From: Andy Lutomirski [mailto:[email protected]]
Sent: Saturday, March 08, 2014 20:58
To: [email protected]
Cc: [email protected]; Andy Lutomirski; Winkler, Tomas
Subject: [PATCH 1/4] mei: Don't log an error when the MEI device is quirked off
If an administrator wants to use MEI, they can search the logs for 'mei'. Otherwise they don't need a glaring reminder that their hardware doesn't actually support MEI.
Signed-off-by Andy Lutomirski <[email protected]>
---
 drivers/misc/mei/pci-me.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/misc/mei/pci-me.c +0 -0
index ddadd08..809021e 100644
@@ -127,7 +127,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
 struct pci_device_id *ent)
 

Quoted 10 lines

drivers/misc/mei/pci-me.c static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent) · line 131
Quoted 5 lines
drivers/misc/mei/pci-me.c +1 -1
      if (!mei_me_quirk_probe(pdev, ent)) {
              err = -ENODEV;
-             goto end;
+             return err;  /* No point in logging an error. */
      }
I would like to have at least some info/debug message logged,
the issue was discovered on specific DELL machines and there is a little worry that on some systems it can be false positive.

mei_me_quirk_probe logs "Device doesn't have valid ME Interface\n".

--Andy

Winkler, Tomas

2014-03-11 22:34 UTC #4 lore raw

RE: [PATCH 1/4] mei: Don't log an error when the MEI device is quirked off

Quoted 30 lines

Quoted 19 lines
Quoted 19 lines
If an administrator wants to use MEI, they can search the logs for 'mei'. Otherwise they don't need a glaring reminder that their hardware doesn't actually support MEI.
Signed-off-by Andy Lutomirski <[email protected]>
---
 drivers/misc/mei/pci-me.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/misc/mei/pci-me.c +0 -0
index ddadd08..809021e 100644
@@ -127,7 +127,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
 struct pci_device_id *ent)
 
Quoted 10 lines
drivers/misc/mei/pci-me.c static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent) · line 131
Quoted 5 lines
drivers/misc/mei/pci-me.c +1 -1
      if (!mei_me_quirk_probe(pdev, ent)) {
              err = -ENODEV;
-             goto end;
+             return err;  /* No point in logging an error. */
      }
I would like to have at least some info/debug message logged,
the issue was discovered on specific DELL machines and there is a little worry that on some systems it can be false positive.

Quoted 1 line

mei_me_quirk_probe logs "Device doesn't have valid ME Interface\n".

Grrr, right!
ACK, though it would be simpler just to:

     if (!mei_me_quirk_probe(pdev, ent))
                     return -ENODEV;

Even this is trivial fix I would prefer it will go through Greg's char-misc tree, as this is what I'm rebasing on.

Thanks
Tomas

Andy Lutomirski

author

2014-03-08 18:58 UTC #5 lore raw

PATCH 2/4 megaraid: Downgrade success messages from KERN_ERR to dev_dbg

In reply to Andy Lutomirski

These messages are uninteresting indications of success.

Signed-off-by Andy Lutomirski <[email protected]>

---
 drivers/scsi/megaraid/megaraid_sas_base.c   | 2 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

drivers/scsi/megaraid/megaraid_sas_base.c +1 -1

index 3b7ad10..a0a133d 100644
@@ -3755,7 +3755,7 @@ static int megasas_init_fw(struct megasas_instance *instance)
 	if (instance->instancet->init_adapter(instance))
 		goto fail_init_adapter;
 
-	printk(KERN_ERR "megasas: INIT adapter done\n");
+	dev_dbg(&instance->pdev->dev, "INIT adapter done\n");
 
 	/** for passthrough
 	* the following function will get the PD LIST.

drivers/scsi/megaraid/megaraid_sas_fusion.c +1 -1

index f655592..261742a 100644
@@ -679,7 +679,7 @@ megasas_ioc_init_fusion(struct megasas_instance *instance)
 		ret = 1;
 		goto fail_fw_init;
 	}
-	printk(KERN_ERR "megasas:IOC Init cmd success\n");
+	dev_dbg(&instance->pdev->dev, "IOC Init cmd success\n");
 
 	ret = 0;

Signature · 1 line

1.8.5.3

Andy Lutomirski

author

2014-03-08 18:58 UTC #6 lore raw

PATCH 3/4 sr: Add a missing KERN_INFO

In reply to Andy Lutomirski

The existence if scsi3-mmc capabilities is not an error.

Signed-off-by Andy Lutomirski <[email protected]>

---
 drivers/scsi/sr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/scsi/sr.c +2 -1

index 40d8592..67baf7f 100644
@@ -887,7 +887,8 @@ static void get_capabilities(struct scsi_cd *cd)
 	cd->readcd_known = 1;
 	cd->readcd_cdda = buffer[n + 5] & 0x01;
 	/* print some capability bits */
-	printk("%s: scsi3-mmc drive: %dx/%dx %s%s%s%s%s%s\n", cd->cdi.name,
+	printk(KERN_INFO "%s: scsi3-mmc drive: %dx/%dx %s%s%s%s%s%s\n",
+	       cd->cdi.name,
 	       ((buffer[n + 14] << 8) + buffer[n + 15]) / 176,
 	       cd->cdi.speed,
 	       buffer[n + 3] & 0x01 ? "writer " : "", /* CD Writer */

Signature · 1 line

1.8.5.3

Andy Lutomirski

author

2014-03-08 18:59 UTC #7 lore raw

PATCH 4/4 raid6: Add severity levels to raid6 initialization messages

In reply to Andy Lutomirski

Some of them are errors; some are not. Annotate them accordingly.

Signed-off-by Andy Lutomirski <[email protected]>

---
 lib/raid6/algos.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

lib/raid6/algos.c +7 -6

index f0b1aa3..6edc821 100644
@@ -121,9 +121,10 @@ static inline const struct raid6_recov_calls *raid6_choose_recov(void)
 		raid6_2data_recov = best->data2;
 		raid6_datap_recov = best->datap;
 
-		printk("raid6: using %s recovery algorithm\n", best->name);
+		printk(KERN_DEBUG "raid6: using %s recovery algorithm\n",
+		       best->name);
 	} else
-		printk("raid6: Yikes! No recovery algorithm found!\n");
+		printk(KERN_ERR "raid6: Yikes! No recovery algorithm found!\n");
 
 	return best;
 }

@@ -157,18 +158,18 @@ static inline const struct raid6_calls *raid6_choose_gen(
 				bestperf = perf;
 				best = *algo;
 			}
-			printk("raid6: %-8s %5ld MB/s\n", (*algo)->name,
+			printk(KERN_DEBUG "raid6: %-8s %5ld MB/s\n", (*algo)->name,
 			       (perf*HZ) >> (20-16+RAID6_TIME_JIFFIES_LG2));
 		}
 	}
 
 	if (best) {
-		printk("raid6: using algorithm %s (%ld MB/s)\n",
+		printk(KERN_DEBUG "raid6: using algorithm %s (%ld MB/s)\n",
 		       best->name,
 		       (bestperf*HZ) >> (20-16+RAID6_TIME_JIFFIES_LG2));
 		raid6_call = *best;
 	} else
-		printk("raid6: Yikes!  No algorithm found!\n");
+		printk(KERN_ERR "raid6: Yikes!  No algorithm found!\n");
 
 	return best;
 }

@@ -194,7 +195,7 @@ int __init raid6_select_algo(void)
 	syndromes = (void *) __get_free_pages(GFP_KERNEL, 1);
 
 	if (!syndromes) {
-		printk("raid6: Yikes!  No memory available.\n");
+		printk(KERN_ERR "raid6: Yikes!  No memory available.\n");
 		return -ENOMEM;
 	}

Signature · 1 line

1.8.5.3

Paul Bolle

2014-03-08 19:14 UTC #8 lore raw

Re: [PATCH 4/4] raid6: Add severity levels to raid6 initialization messages

Quoted 10 lines · Andy Lutomirski

Some of them are errors; some are not. Annotate them accordingly.
Signed-off-by Andy Lutomirski <[email protected]>
---
 lib/raid6/algos.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
lib/raid6/algos.c +0 -0
[...]

lib/raid6/algos.c int __init raid6_select_algo(void) · line 201

Quoted 8 lines

lib/raid6/algos.c +1 -1

@@ -194,7 +195,7 @@ int __init raid6_select_algo(void)
 	syndromes = (void *) __get_free_pages(GFP_KERNEL, 1);
 
 	if (!syndromes) {
-		printk("raid6: Yikes!  No memory available.\n");
+		printk(KERN_ERR "raid6: Yikes!  No memory available.\n");
 		return -ENOMEM;
 	}

Does __get_free_pages() print and error (or similar) when it fails? If so, this printk() could be dropped, couldn't it?

Paul Bolle

Andy Lutomirski

author

2014-03-11 22:22 UTC #9 lore raw

Re: [PATCH 4/4] raid6: Add severity levels to raid6 initialization messages

Quoted 13 lines · Paul Bolle

Quoted 12 lines · Andy Lutomirski
Some of them are errors; some are not. Annotate them accordingly.
Signed-off-by Andy Lutomirski <[email protected]>
---
 lib/raid6/algos.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
lib/raid6/algos.c +0 -0
[...]
@@ -194,7 +195,7 @@ int __init raid6_select_algo(void)
      syndromes = (void *) __get_free_pages(GFP_KERNEL, 1);
 

Quoted 9 lines

lib/raid6/algos.c int __init raid6_select_algo(void) · line 201
Quoted 6 lines
lib/raid6/algos.c +1 -1
      if (!syndromes) {
-             printk("raid6: Yikes!  No memory available.\n");
+             printk(KERN_ERR "raid6: Yikes!  No memory available.\n");
              return -ENOMEM;
      }
 
Does __get_free_pages() print and error (or similar) when it fails? If so, this printk() could be dropped, couldn't it?

I think it does in warn_alloc_failed in mm/page_alloc.c. It might not be clear what the impact of that error is, but, empirically, if there's too little memory during boot, everything is thoroughly broken anyway.

Quoted 4 lines

Paul Bolle

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC