On 11/14/2014 11:53 PM, Elliott, Robert (Server Storage) wrote: > > >> -----Original Message----- >> From: Hannes Reinecke [mailto:h...@suse.de] >> Sent: Thursday, 06 November, 2014 2:31 AM >> To: James Bottomley >> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage); >> linux-scsi@vger.kernel.org; Hannes Reinecke >> Subject: [PATCH 10/10] scsi: Do not display buffer pointers in >> scsi_log_send() >> >> scsi_log_send() would display buffer pointer for higher >> logging levels. This is not only of questionable value >> but also exposes kernel pointer to userspace, which is >> discouraged in some setups. So drop this message >> altogether. >> >> Signed-off-by: Hannes Reinecke <h...@suse.de> >> --- >> drivers/scsi/scsi.c | 9 +-------- >> 1 file changed, 1 insertion(+), 8 deletions(-) >> >> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c >> index 92d5912..9ec576d 100644 >> --- a/drivers/scsi/scsi.c >> +++ b/drivers/scsi/scsi.c >> @@ -531,7 +531,7 @@ void scsi_log_send(struct scsi_cmnd *cmd) >> * >> * 3: same as 2 >> * >> - * 4: same as 3 plus dump extra junk >> + * 4: same as 3 >> */ >> if (unlikely(scsi_logging_level)) { >> level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT, >> @@ -540,13 +540,6 @@ void scsi_log_send(struct scsi_cmnd *cmd) >> scmd_printk(KERN_INFO, cmd, >> "Send: scmd 0x%p\n", cmd); >> scsi_print_command(cmd); >> - if (level > 3) { >> - printk(KERN_INFO "buffer = 0x%p, bufflen = >> %d," >> - " queuecommand 0x%p\n", >> - scsi_sglist(cmd), scsi_bufflen(cmd), >> - cmd->device->host->hostt- >>> queuecommand); >> - >> - } >> } >> } >> } > > Reviewed-by: Robert Elliott <elli...@hp.com> > > One more comment on the series: > > After the tags, there are still a few scmd %p prints left > including the one above. Is this series supposed to get > rid of the rest of them? > > drivers/scsi/scsi.c: "Send: scmd 0x%p\n", cmd); > drivers/scsi/scsi_error.c: "scmd %p eh > timeout, not aborting\n", > drivers/scsi/scsi_error.c: "aborting command > %p\n", scmd)); > drivers/scsi/scsi_error.c: > "scmd %p eh timeout, " > drivers/scsi/scsi_error.c: > "scmd %p retry " > drivers/scsi/scsi_error.c: > "scmd %p finish " > drivers/scsi/scsi_error.c: "scmd %p > abort %s\n", scmd, > drivers/scsi/scsi_error.c: "scmd %p > terminate " > drivers/scsi/scsi_error.c: "scmd %p previous > abort failed\n", scmd)); > drivers/scsi/scsi_error.c: "scmd %p not > aborting, host in recovery\n", > drivers/scsi/scsi_error.c: "scmd %p abort > scheduled\n", scmd)); > drivers/scsi/scsi_error.c: "%s scmd: %p result: %x\n", > drivers/scsi/scsi_error.c: "%s: scmd: %p, timeleft: > %ld\n", > drivers/scsi/scsi_error.c: "sense requested for %p > result %x\n", > drivers/scsi/scsi_error.c: "%s: scmd %p rtn %x\n", __func__, > scmd, rtn)); > drivers/scsi/scsi_error.c: "%s: > flush retry cmd: %p\n", > drivers/scsi/scsi_error.c: "%s: > flush finish cmd: %p\n", > drivers/scsi/scsi_lib.c: "Inserting command %p into > mlqueue\n", cmd)); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s no > slave_alloc function in hostt %p\n", __func__, shost->hostt); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "sdev %p scmd %p > sending INQUIRY\n", sdev, scsi_cmd); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from > lookup_by_target\n", __func__, sdev); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from > alloc_sdev\n", __func__, sdev); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from > alloc_sdev\n", __func__, sdev); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from > lookup_by_target\n", __func__, sdev); > drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from > alloc_sdev\n", __func__, sdev); > Ideally, yes. However, for this to work we need an additional means of identifying the command uniquely. With the current infrastructure 'tags' are only ever assigned for devices using the SCSI TCQ mechanism. Everything else is left in the dark, even though it might be allowing for a queue depths > 1. So I'll need to revisit this now that the tag rework from Christoph Hellwig is in. But I'd rather have this postponed to a next patch series. That series would also included the proposed 'SUCCESS' to 'COMPLETE' change.
Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
