Subject: Re: [PATCH v4 12/32] cxlflash: Fix to avoid spamming the kernel log

From: Andrew Donnellan <[email-masked]>
Date: Tue, 29 Sep 2015 15:05:55 +1000

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 <[email-masked]>
> Signed-off-by: Manoj N. Kumar <[email-masked]>
> Reviewed-by: Brian King <[email-masked]>

Reviewed-by: Andrew Donnellan <[email-masked]>

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.

> @@ -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?

Andrew

-- 
Andrew Donnellan              Software Engineer, OzLabs
[email-masked]  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited