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

2015-09-29 Thread Matthew R. Ochs 
> On Sep 29, 2015, at 12:05 AM, Andrew Donnellan  
> 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 
>> Signed-off-by: Manoj N. Kumar 
>> Reviewed-by: Brian King 
> 
> Reviewed-by: Andrew Donnellan 
> 
> 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.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-09-28 Thread Andrew Donnellan 

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


Reviewed-by: Andrew Donnellan 

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
andrew.donnel...@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)IBM Australia Limited

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-09-25 Thread Matthew R. Ochs 
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 
Signed-off-by: Manoj N. Kumar 
Reviewed-by: Brian King 

Conflicts:
drivers/scsi/cxlflash/main.c
---
 drivers/scsi/cxlflash/main.c | 109 +++
 1 file changed, 59 insertions(+), 50 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b44212b..527ff85 100644
--- 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);
memset(cmd->buf, 0, CMD_BUFSIZE);
memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
return cmd;
@@ -93,7 +93,7 @@ static void cmd_checkin(struct afu_cmd *cmd)
return;
}
 
-   pr_debug("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
+   pr_devel("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
 }
 
 /**
@@ -127,7 +127,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct 
scsi_cmnd *scp)
}
 
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);
@@ -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);
@@ -417,12 +417,13 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd 
*scp, u64 tmfcmd)
short lflag = 0;
struct Scsi_Host *host = scp->device->host;
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
+   struct device *dev = &cfg->dev->dev;
ulong lock_flags;
int rc = 0;
 
cmd = cmd_checkout(afu);
if (unlikely(!cmd)) {
-   pr_err("%s: could not get a free command\n", __func__);
+   dev_err(dev, "%s: could not get a free command\n", __func__);
rc = SCSI_MLQUEUE_HOST_BUSY;
goto out;
}
@@ -493,7 +494,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
 {
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
struct afu *afu = cfg->afu;
-   struct pci_dev *pdev = cfg->dev;
+   struct device *dev = &cfg->dev->dev;
struct afu_cmd *cmd;
u32 port_sel = scp->device->channel + 1;
int nseg, i, ncount;
@@ -502,13 +503,14 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
short lflag = 0;
int rc = 0;
 
-   pr_debug("%s: (scp=%p) %d/%d/%d/%llu cdb=(%08X-%08X-%08X-%08X)\n",
-__func__, scp, host->host_no, scp->device->channel,
-scp->device->id, scp->device->lun,
-get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+   dev_dbg_ratelimited(dev, "%s: (scp=%p) %d/%d/%d/%llu "
+   "cdb=(%08X-%08X-%08X-%08X)\n",
+   __func__, scp, host->host_no, scp->device->channel,
+   scp->device->id, scp->device->lun,
+   get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
+   get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
+   get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
+   get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
 
/* If