On 09/05/2014 04:04 AM, Yoshihiro YUNOMAE wrote: > (2014/09/03 19:06), Hannes Reinecke wrote: >> Implement scsi_retval_string() to simplify logging. >> >> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> >> --- >> drivers/scsi/constants.c | 28 ++++++++++++++++++++++++++++ >> drivers/scsi/scsi.c | 34 ++++++---------------------------- >> include/scsi/scsi_dbg.h | 1 + >> 3 files changed, 35 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c >> index 5486816..85d2da0 100644 >> --- a/drivers/scsi/constants.c >> +++ b/drivers/scsi/constants.c >> @@ -1426,6 +1426,34 @@ void scsi_print_sense(struct scsi_cmnd *cmd) >> EXPORT_SYMBOL(scsi_print_sense); >> >> #ifdef CONFIG_SCSI_CONSTANTS >> +static const struct error_info internal_retval_table[] = >> +{ >> + { NEEDS_RETRY, "NEEDS_RETRY " }, >> + { SUCCESS, "SUCCESS " }, >> + { FAILED, "FAILED " }, >> + { QUEUED, "QUEUED " }, >> + { SOFT_ERROR, "SOFT_ERROR " }, >> + { ADD_TO_MLQUEUE, "ADD_TO_MLQUEUE " }, >> + { TIMEOUT_ERROR, "TIMEOUT " }, >> + { SCSI_RETURN_NOT_HANDLED, "NOT_HANDLED " }, >> + { FAST_IO_FAIL, "FAST_IO_FAIL " }, > > We don't need to add space in the last of strings, I think. > In scsi_log_completion(), the messages inserts line feeds after the > strings. > >> +}; >> +#endif >> + >> +const char * >> +scsi_retval_string(unsigned int ret) >> +{ >> +#ifdef CONFIG_SCSI_CONSTANTS >> + int i; >> + >> + for (i = 0; internal_retval_table[i].text; i++) >> + if (internal_retval_table[i].code12 == ret) >> + return internal_retval_table[i].text; >> +#endif >> + return NULL; >> +} >> + >> +#ifdef CONFIG_SCSI_CONSTANTS >> >> static const char * const hostbyte_table[]={ >> "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET", >> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c >> index 8954036..a1944c8 100644 >> --- a/drivers/scsi/scsi.c >> +++ b/drivers/scsi/scsi.c >> @@ -566,35 +566,13 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) >> SCSI_LOG_MLCOMPLETE_BITS); >> if (((level > 0) && (cmd->result || disposition != SUCCESS)) || >> (level > 1)) { >> - scmd_printk(KERN_INFO, cmd, "Done: "); >> if (level > 2) >> - printk("0x%p ", cmd); >> - /* >> - * Dump truncated values, so we usually fit within >> - * 80 chars. >> - */ >> - switch (disposition) { >> - case SUCCESS: >> - printk("SUCCESS\n"); >> - break; >> - case NEEDS_RETRY: >> - printk("RETRY\n"); >> - break; >> - case ADD_TO_MLQUEUE: >> - printk("MLQUEUE\n"); >> - break; >> - case FAILED: >> - printk("FAILED\n"); >> - break; >> - case TIMEOUT_ERROR: >> - /* >> - * If called via scsi_times_out. >> - */ >> - printk("TIMEOUT\n"); >> - break; >> - default: >> - printk("UNKNOWN\n"); >> - } >> + scmd_printk(KERN_INFO, cmd, >> + "Done: 0x%p %s\n", cmd, >> + scsi_retval_string(disposition)); >> + else >> + scmd_printk(KERN_INFO, cmd, "Done: %s", > > We had better add "\n" in this last strings to indicate the end of line. > Structured printk automatically outputs the message in atomic, > but adding "\n" becomes more readable. > True, we should be adding the newline. Although the reasoning is wrong; a newline instructs printk() to actually ship out the message. Without a newline printk assumes it'll be line continuation. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html