Re: [PATCH] pm8001: panics/lockups from asynchronous device removal (take 2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the review.

On Jan 17, 2012, at 8:37 PM, Jack Wang wrote:

> Here KERN_LEVEL is wrong, maybe you means KERN_INFO, others is good, thanks for fix this.

Sigh ... checkpatch complained, I edited the patch directly as I thought it was obvious, but reading checkpatch rather than the code. I need to add a required compile-check on a final-patch (or stop editing patches; edit code only, generate patches) before submittal in my process. update follows:


pm8001_query_task and pm8001_abort_task panic kernel when devices asynchronously disappear, a possible scenario since these functions are generally called when errors are mounting. Some of the panics are a direct result of a failure to NULL check some of the structure variables that are in certain states of teardown. One of the lockups was a direct result of returning an unexpected code to libsas' sas_scsi_find_task() function (creating a tight loop of an unexpected code 138 upstream to the scsi layers queue function).

Signed-off-by: mark_salyzyn@xxxxxxxxxxx
Cc: jack_wang@xxxxxxxxx
Cc: JBottomley@xxxxxxxxxxxxx
Cc: crystal_yu@xxxxxxxxx
Cc: john_gong@xxxxxxxxx
Cc: lindar_liu <lindar_liu@xxxxxxxxx>

 drivers/csi/pm8001/pm8001_sas.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index fb3dc99..1cac756 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -940,6 +940,8 @@ int pm8001_query_task(struct sas_task *task)
 		struct pm8001_hba_info *pm8001_ha =
 			pm8001_find_ha_by_dev(dev);
 
+		if (unlikely(!cmnd || !cmnd->device))
+			return rc;
 		int_to_scsilun(cmnd->device->lun, &lun);
 		rc = pm8001_find_tag(task, &tag);
 		if (rc == 0) {
@@ -947,9 +949,11 @@ int pm8001_query_task(struct sas_task *task)
 			return rc;
 		}
 		PM8001_EH_DBG(pm8001_ha, pm8001_printk("Query:["));
-		for (i = 0; i < 16; i++)
-			printk(KERN_INFO "%02x ", cmnd->cmnd[i]);
-		printk(KERN_INFO "]\n");
+		if (pm8001_ha->logging_level & PM8001_EH_LOGGING) {
+			for (i = 0; i < 16; i++)
+				printk(KERN_INFO "%02x ", cmnd->cmnd[i]);
+			printk(KERN_INFO "]\n");
+		}
 		tmf_task.tmf = 	TMF_QUERY_TASK;
 		tmf_task.tag_of_task_to_be_managed = tag;
 
@@ -960,6 +964,13 @@ int pm8001_query_task(struct sas_task *task)
 			PM8001_EH_DBG(pm8001_ha,
 				pm8001_printk("The task is still in Lun\n"));
 			break;
+
+		/* libsas can not handle any other error code than this list */
+		default:
+			/* rc is likely something like SAS_OPEN_REJECT etc */
+			pm8001_printk("pm8001_issue_ssp_tmf()=0x%x\n", rc);
+			rc = TMF_RESP_FUNC_FAILED;
+			/* FALLTHRU */
 		/* The task is not in Lun or failed, reset the phy */
 		case TMF_RESP_FUNC_FAILED:
 		case TMF_RESP_FUNC_COMPLETE:
@@ -986,7 +997,8 @@ int pm8001_abort_task(struct sas_task *task)
 	struct pm8001_device *pm8001_dev;
 	struct pm8001_tmf_task tmf_task;
 	int rc = TMF_RESP_FUNC_FAILED;
-	if (unlikely(!task || !task->lldd_task || !task->dev))
+	if (unlikely(!task || !task->lldd_task
+	 || !task->dev || !task->dev->lldd_dev))
 		return rc;
 	spin_lock_irqsave(&task->task_state_lock, flags);
 	if (task->task_state_flags & SAS_TASK_STATE_DONE) {
@@ -1001,6 +1013,8 @@ int pm8001_abort_task(struct sas_task *task)
 		ccb = task->lldd_task;
 		pm8001_dev = dev->lldd_dev;
 		pm8001_ha = pm8001_find_ha_by_dev(dev);
+		if (unlikely(!cmnd || !cmnd->device))
+			return rc;
 		int_to_scsilun(cmnd->device->lun, &lun);
 		rc = pm8001_find_tag(task, &tag);
 		if (rc == 0) {

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


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux