Re: [PATCH 16/20] scsi: separate out scsi_retval_string()

Hannes Reinecke
2014-09-05T06:14:21Z

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@suse.de>
>> ---
>> 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@suse.de +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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html