scsi: smartpqi_init: Reporting 'logical unit failure'

When this HARDWARE_ERROR/0x3e/0x1 case is triggered, the logical volume is offlined.
When reading the kernel log, the cause why the device got offlined isn't reported to the user.
This situation makes difficult for admins to estimate _why_ the volume got offlined.
Reading this part of the code makes clear this is because driver received a HARDWARE_ERROR/0x3e/0x1 which is a 'logical unit failure'.

This patch is just about reporting that fact to help admins making a relationship between this event and the offlining.

Signed-off-by Erwan Velu <[email protected]>

---
  drivers/scsi/smartpqi/smartpqi_init.c | 5 +++++
  1 file changed, 5 insertions(+)

drivers/scsi/smartpqi/smartpqi_init.c +6 -0

index f564af8949e8..89f37d76735c 100644
@@ -2764,6 +2764,12 @@ static void pqi_process_raid_io_error(struct pqi_io_request *io_request)
  				sshdr.sense_key == HARDWARE_ERROR &&
  				sshdr.asc == 0x3e &&
  				sshdr.ascq == 0x1) {
+			struct pqi_ctrl_info *ctrl_info = shost_to_hba(scmd->device->host);
+			struct pqi_scsi_dev *device = scmd->device->hostdata;
+
+			dev_err(&ctrl_info->pci_dev->dev, "received 'logical unit failure' from controller for scsi %d:%d:%d:%d\n",
+							ctrl_info->scsi_host->host_no, device->bus,
+							device->target, device->lun);
  			pqi_take_device_offline(scmd->device, "RAID");
  			host_byte = DID_NO_CONNECT;
  		}

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Erwan Velu
Sent: Wednesday, February 27, 2019 10:32 AM
Subject: [PATCH] scsi: smartpqi_init: Reporting 'logical unit failure'

When this HARDWARE_ERROR/0x3e/0x1 case is triggered, the logical volume is offlined. When reading the kernel log, the cause why the device got offlined isn't reported to the user. This situation makes difficult for admins to estimate _why_ the volume got offlined. Reading this part of the code makes clear this is because driver received a HARDWARE_ERROR/0x3e/0x1 which is a 'logical unit failure'.

This patch is just about reporting that fact to help admins making a relationship between this event and the offlining.

Signed-off-by Erwan Velu <[email protected]>

---
 drivers/scsi/smartpqi/smartpqi_init.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/scsi/smartpqi/smartpqi_init.c +6 -0

index f564af8949e8..89f37d76735c 100644
@@ -2764,6 +2764,12 @@ static void pqi_process_raid_io_error(struct pqi_io_request *io_request)
 				sshdr.sense_key == HARDWARE_ERROR &&
 				sshdr.asc == 0x3e &&
 				sshdr.ascq == 0x1) {
+			struct pqi_ctrl_info *ctrl_info = shost_to_hba(scmd->device->host);
+			struct pqi_scsi_dev *device = scmd->device->hostdata;
+
+			dev_err(&ctrl_info->pci_dev->dev, "received 'logical unit failure' from controller
 for scsi %d:%d:%d:%d\n",
+							ctrl_info->scsi_host->host_no, device->bus,
+							device->target, device->lun);
 			pqi_take_device_offline(scmd->device, "RAID");
 			host_byte = DID_NO_CONNECT;

}

Be careful printing errors per-IO; you could get thousands of them if things go bad. The block layer print_req_error() uses printk_ratelimited(KERN_ERR) for that reason, and the SCSI layer scsi_io_completion_action() maintains a ratelimit on its own.

The dev_err_ratelimited() macro might be a good fit here.

(quoted diff) +1 -0

+			dev_err_ratelimited(&ctrl_info->pci_dev-

Quoted 1 line

dev, "received 'logical unit failure' from controller for scsi

%d:%d:%d:%d\n",

(quoted diff) +1 -0

+							ctrl_info-

Quoted 1 line

scsi_host->host_no, device->bus,

(quoted diff) +1 -0

+							device-

Quoted 1 line

target, device->lun);

[...]
Shouldn't this be a variant of sdev/scmd_printk? Otherwise it tells you what disk in the array terms is the problem but not what device in your actual system is affected.

Quoted 6 lines · James Bottomley

[...]
Shouldn't this be a variant of sdev/scmd_printk? Otherwise it tells
you what disk in the array terms is the problem but not what device in
your actual system is affected.

Hey James,

My initial take on that was that pqi_take_device_offline(), which is called just after, will print the "re-scanning " message with the same
format.

As they will be both printed in the same error context and one after the
other, I though that would make sense to represent the same information
to ease the reading like cause -> consequence.

As the message is about the LUN itself, which is reported faulty, I though it would worth reporting the info that way.

Shall I consider printing also the disk name in addition ?

[...]
I was thinking just

if (printk_ratelimit())
scmd_printk(KERN_ERR, scmd, "received 'logical unit failure' from controller for scsi %d:%d:%d:%d\n", ...

That will give all the necessary information

When the HARDWARE_ERROR/0x3e/0x1 case is triggered, the logical volume is offlined. When reading the kernel log, the reason why the device got offlined isn't reported to the user. This situation makes difficult for admins to estimate the root cause of the issue they analize.

Erwan,

Quoted 5 lines

When the HARDWARE_ERROR/0x3e/0x1 case is triggered, the logical volume is offlined. When reading the kernel log, the reason why the device got offlined isn't reported to the user. This situation makes difficult for admins to estimate the root cause of the issue they analize.

Applied to 5.1/scsi-queue, thanks.

Erwan,

Quoted 5 lines

When the HARDWARE_ERROR/0x3e/0x1 case is triggered, the logical volume is offlined. When reading the kernel log, the reason why the device got offlined isn't reported to the user. This situation makes difficult for admins to estimate the root cause of the issue they analize.