> On Sep 29, 2015, at 12:05 AM, Andrew Donnellan <andrew.donnellan@xxxxxxxxxxx> wrote:
> On 26/09/15 09:15, Matthew R. Ochs wrote:
>> During run-time the driver can be very chatty and spam the system
>> kernel log. Various print statements can be limited and/or moved
>> to development-only mode. Additionally, numerous prints can be
>> converted to trace the corresponding device.
>>
>> The following changes were made:
>> - pr_debug to pr_devel
>> - pr_debug to pr_debug_ratelimited
>> - pr_err to dev_err
>> - pr_debug to dev_dbg
>>
>> Signed-off-by: Matthew R. Ochs <mrochs@xxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Manoj N. Kumar <manoj@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Brian King <brking@xxxxxxxxxxxxxxxxxx>
>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@xxxxxxxxxxx>
>
> Changes mostly look fine, further comments below.
>
>> --- a/drivers/scsi/cxlflash/main.c
>> +++ b/drivers/scsi/cxlflash/main.c
>> @@ -58,8 +58,8 @@ static struct afu_cmd *cmd_checkout(struct afu *afu)
>> cmd = &afu->cmd[k];
>>
>> if (!atomic_dec_if_positive(&cmd->free)) {
>> - pr_debug("%s: returning found index=%d\n",
>> - __func__, cmd->slot);
>> + pr_devel("%s: returning found index=%d cmd=%p\n",
>> + __func__, cmd->slot, cmd);
>
>> pr_debug("%s: cmd failed afu_rc=%d scsi_rc=%d fc_rc=%d "
>> - "afu_extra=0x%X, scsi_entra=0x%X, fc_extra=0x%X\n",
>> + "afu_extra=0x%X, scsi_extra=0x%X, fc_extra=0x%X\n",
>> __func__, ioasa->rc.afu_rc, ioasa->rc.scsi_rc,
>> ioasa->rc.fc_rc, ioasa->afu_extra, ioasa->scsi_extra,
>> ioasa->fc_extra);
>
> Minor nitpicking: mention that you fix these in the commit message.
Noted for the future.
>> @@ -240,9 +240,9 @@ static void cmd_complete(struct afu_cmd *cmd)
>> cmd_is_tmf = cmd->cmd_tmf;
>> cmd_checkin(cmd); /* Don't use cmd after here */
>>
>> - pr_debug("%s: calling scsi_set_resid, scp=%p "
>> - "result=%X resid=%d\n", __func__,
>> - scp, scp->result, resid);
>> + pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "
>> + "ioasc=%d\n", __func__, scp, scp->result,
>> + cmd->sa.ioasc);
>>
>> scsi_set_resid(scp, resid);
>> scsi_dma_unmap(scp);
>
> Why has the message changed from scsi_set_resid to scsi_done, and should the message be moved to immediately before the scsi_done call?
In a later patch in the series the scsi_set_resid() is actually moved.
--
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
