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

[Hannes Reinecke <hare@suse.de>]

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