Skip to contentSkip to conversation

Only print SCSI data direction warning once for a command

PATCH In review Andi Kleen 3 participants 4 messages Jan 2, 2008 – Jan 15, 2008

Subsystems (2)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 Only print SCSI data direction warning once for a command

When I use cdparanoia my logs get spammed a lot by

printk: 464 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
program cdparanoia not setting count and/or reply_len properly
printk: 1078 messages suppressed.

and many more of those. With this patch the message is only printed once for a command in a row.

---
 drivers/scsi/sg.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/scsi/sg.c +6 -2
Index: linux/drivers/scsi/sg.c
===================================================================
@@ -602,8 +602,9 @@ sg_write(struct file *filp, const char _
 	 * but is is possible that the app intended SG_DXFER_TO_DEV, because there
 	 * is a non-zero input_size, so emit a warning.
 	 */
-	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV)
-		if (printk_ratelimit())
+	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
+		static char cmd[TASK_COMM_LEN];
+		if (printk_ratelimit() && strcmp(current->comm, cmd)) {
 			printk(KERN_WARNING
 			       "sg_write: data in/out %d/%d bytes for SCSI command 0x%x--"
 			       "guessing data in;\n" KERN_WARNING "   "
@@ -611,6 +612,9 @@ sg_write(struct file *filp, const char _
 			       old_hdr.reply_len - (int)SZ_SG_HEADER,
 			       input_size, (unsigned int) cmnd[0],
 			       current->comm);
+			strcpy(cmd, current->comm);
+		}
+	}
 	k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
 	return (k < 0) ? k : count;
 }
maintainer
#1 lore raw
Quoted 9 lines · Andi Kleen

When I use cdparanoia my logs get spammed a lot by

printk: 464 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
program cdparanoia not setting count and/or reply_len properly
printk: 1078 messages suppressed.

and many more of those. With this patch the message is only printed once for a command in a row.

My reaction is that the intent of these warnings is to try to get people to fix broken applications, so I'm not sure any action is appropriate; however, it's Doug's driver, so I'll defer to him.

Even if he does say yes, though, your patch looks wrong. It's still going to spew the

printk: 1078 messages suppressed.

to the log because they come from printk_ratelimit(). So all you've done is halved the volume of flow to the logs and left a dangling printk suppressed message that keeps spewing, so I don't think the patch even does what you describe it as doing ... if you reverse the order of the operands in the if() it will ...

James

author
#2 lore raw
Quoted 6 lines

to the log because they come from printk_ratelimit(). So all you've done is halved the volume of flow to the logs and left a dangling printk suppressed message that keeps spewing, so I don't think the patch even does what you describe it as doing ... if you reverse the order of the operands in the if() it will ...

Here's an updated patch with the reversed order.

-Andi

---

Only print SCSI data direction warning once for a command v2

When I use cdparanoia my logs get spammed a lot by

printk: 464 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
program cdparanoia not setting count and/or reply_len properly
printk: 1078 messages suppressed.

and many more of those. With this patch the message is only printed once for a command in a row.

v1->v2: Prevent rate limit messages too (pointed out by jejb)

---
 drivers/scsi/sg.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/scsi/sg.c +6 -2
Index: linux/drivers/scsi/sg.c
===================================================================
@@ -602,8 +602,9 @@ sg_write(struct file *filp, const char _
 	 * but is is possible that the app intended SG_DXFER_TO_DEV, because there
 	 * is a non-zero input_size, so emit a warning.
 	 */
-	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV)
-		if (printk_ratelimit())
+	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
+		static char cmd[TASK_COMM_LEN];
+		if (strcmp(current->comm, cmd) && printk_ratelimit()) {
 			printk(KERN_WARNING
 			       "sg_write: data in/out %d/%d bytes for SCSI command 0x%x--"
 			       "guessing data in;\n" KERN_WARNING "   "
@@ -611,6 +612,9 @@ sg_write(struct file *filp, const char _
 			       old_hdr.reply_len - (int)SZ_SG_HEADER,
 			       input_size, (unsigned int) cmnd[0],
 			       current->comm);
+			strcpy(cmd, current->comm);
+		}
+	}
 	k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
 	return (k < 0) ? k : count;
 }
#3 lore raw
Quoted 51 lines · Andi Kleen
Quoted 5 lines

to the log because they come from printk_ratelimit(). So all you've done is halved the volume of flow to the logs and left a dangling printk suppressed message that keeps spewing, so I don't think the patch even does what you describe it as doing ... if you reverse the order of the operands in the if() it will ...

Here's an updated patch with the reversed order.

-Andi

---

Only print SCSI data direction warning once for a command v2

When I use cdparanoia my logs get spammed a lot by

printk: 464 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
program cdparanoia not setting count and/or reply_len properly
printk: 1078 messages suppressed.

and many more of those. With this patch the message is only printed once for a command in a row.

v1->v2: Prevent rate limit messages too (pointed out by jejb)

---
 drivers/scsi/sg.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux/drivers/scsi/sg.c
===================================================================

drivers/scsi/sg.c +3 -2
@@ -602,8 +602,9 @@ sg_write(struct file *filp, const char _
 	 * but is is possible that the app intended SG_DXFER_TO_DEV, because there
 	 * is a non-zero input_size, so emit a warning.
 	 */
-	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV)
-		if (printk_ratelimit())
+	if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
+		static char cmd[TASK_COMM_LEN];
+		if (strcmp(current->comm, cmd) && printk_ratelimit()) {
 			printk(KERN_WARNING
 			       "sg_write: data in/out %d/%d bytes for SCSI command 0x%x--"
 			       "guessing data in;\n" KERN_WARNING "   "

drivers/scsi/sg.c sg_write(struct file *filp, const char _ · line 620

Quoted 9 lines
drivers/scsi/sg.c +3 -0
@@ -611,6 +612,9 @@ sg_write(struct file *filp, const char _
 			       old_hdr.reply_len - (int)SZ_SG_HEADER,
 			       input_size, (unsigned int) cmnd[0],
 			       current->comm);
+			strcpy(cmd, current->comm);
+		}
+	}
 	k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
 	return (k < 0) ? k : count;
 }