On 01/15/2014 11:39 PM, Stephen M. Cameron wrote:
> From: Stephen M. Cameron <scameron@xxxxxxxxxxxxxxxxxx>
>
> On encountering unexpected error conditions from driver initiated
> commands, print something useful like CDB and sense data rather than
> something useless like the kernel virtual address of the command buffer.
>
> Signed-off-by: Stephen M. Cameron <scameron@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/scsi/hpsa.c | 71 ++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 6a50a83..d469974 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -1847,17 +1847,37 @@ static void hpsa_scsi_do_simple_cmd_with_retry(struct ctlr_info *h,
> hpsa_pci_unmap(h->pdev, c, 1, data_direction);
> }
>
> -static void hpsa_scsi_interpret_error(struct CommandList *cp)
> +static void hpsa_print_cmd(struct ctlr_info *h, char *txt,
> + struct CommandList *c)
> {
> - struct ErrorInfo *ei;
> + const u8 *cdb = c->Request.CDB;
> + const u8 *lun = c->Header.LUN.LunAddrBytes;
> +
> + dev_warn(&h->pdev->dev, "%s: LUN:%02x%02x%02x%02x%02x%02x%02x%02x"
> + " CDB:%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
> + txt, lun[0], lun[1], lun[2], lun[3],
> + lun[4], lun[5], lun[6], lun[7],
> + cdb[0], cdb[1], cdb[2], cdb[3],
> + cdb[4], cdb[5], cdb[6], cdb[7],
> + cdb[8], cdb[9], cdb[10], cdb[11],
> + cdb[12], cdb[13], cdb[14], cdb[15]);
> +}
Do you _really_ have to do this?
We do have scsi_logging_level, which would provide exactly this
information.
Can't you rely on the SCSI stack for logging here and drop all
the redundancy in the driver?
Thing is, the hpsa driver is already quite chatty:
hpsa 0000:4f:00.0: Direct-Access device c0b0t0l0 added.
hpsa 0000:4f:00.0: RAID device c0b3t0l0 added.
scsi 0:0:0:0: Direct-Access HP LOGICAL VOLUME 7.24 PQ
: 0 ANSI: 5
sd 0:0:0:0: [sda] 286677120 512-byte logical blocks: (146 GB/136
GiB)
scsi 0:3:0:0: RAID HP P400 7.24 PQ: 0
ANSI: 0
So where is the point of the first two messages?
The enumeration is internal anyway, so it has a very quesionable
value to the user. And the devices are listed afterwards anyway.
Hence it _might_ be an idea to remove some of those messages ...
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
