[SCSI] pm80xx: Spinlock fix
spin_lock_irqsave for the HBA lock is called in one function where flag
is local to that function. Another function is called from the first
function where lock has to be released using spin_unlock_irqrestore for
calling task_done of libsas. In the second function also flag is declared
and used. For calling task_done there is no need to enable the irq. So
instead of using spin_lock_irqsave and spin_unlock_irqrestore, spin_lock
and spin_unlock is used now. This also avoids passing the flags across all
the functions where HBA lock is being used. Also removed redundant code.
Reported-by: Jason Seba <jason.seba42@gmail.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Suresh Thiagarajan <Suresh.Thiagarajan@pmcs.com>
Signed-off-by: Viswas G <viswas.g@pmcs.com>
Acked-by: Jack Wang <xjtuwjp@gmail.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 9b54142..d70587f 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2168,11 +2168,7 @@
IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
ts->resp = SAS_TASK_UNDELIVERED;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*in order to force CPU ordering*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2188,11 +2184,7 @@
IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
ts->resp = SAS_TASK_UNDELIVERED;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2214,11 +2206,7 @@
IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY);
ts->resp = SAS_TASK_UNDELIVERED;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/* ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2281,11 +2269,7 @@
IO_DS_NON_OPERATIONAL);
ts->resp = SAS_TASK_UNDELIVERED;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2305,11 +2289,7 @@
IO_DS_IN_ERROR);
ts->resp = SAS_TASK_UNDELIVERED;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2338,20 +2318,9 @@
" resp 0x%x stat 0x%x but aborted by upper layer!\n",
t, status, ts->resp, ts->stat));
pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- } else if (t->uldd_task) {
+ } else {
spin_unlock_irqrestore(&t->task_state_lock, flags);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/* ditto */
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
- } else if (!t->uldd_task) {
- spin_unlock_irqrestore(&t->task_state_lock, flags);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
}
}
@@ -2463,11 +2432,7 @@
IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_QUEUE_FULL;
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
return;
}
break;
@@ -2589,20 +2554,9 @@
" resp 0x%x stat 0x%x but aborted by upper layer!\n",
t, event, ts->resp, ts->stat));
pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- } else if (t->uldd_task) {
+ } else {
spin_unlock_irqrestore(&t->task_state_lock, flags);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/* ditto */
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
- } else if (!t->uldd_task) {
- spin_unlock_irqrestore(&t->task_state_lock, flags);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- t->task_done(t);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
}
}
@@ -4297,23 +4251,11 @@
"\n", task, ts->resp, ts->stat));
pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
return 0;
- } else if (task->uldd_task) {
+ } else {
spin_unlock_irqrestore(&task->task_state_lock,
flags);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
- mb();/* ditto */
- spin_unlock_irq(&pm8001_ha->lock);
- task->task_done(task);
- spin_lock_irq(&pm8001_ha->lock);
- return 0;
- } else if (!task->uldd_task) {
- spin_unlock_irqrestore(&task->task_state_lock,
- flags);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
- mb();/*ditto*/
- spin_unlock_irq(&pm8001_ha->lock);
- task->task_done(task);
- spin_lock_irq(&pm8001_ha->lock);
+ pm8001_ccb_task_free_done(pm8001_ha, task,
+ ccb, tag);
return 0;
}
}