@@ -207,6 +207,7 @@ static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
struct hisi_sas_slot *slot, bool need_lock)
{
+ unsigned long flags;
int device_id = slot->device_id;
struct hisi_sas_device *sas_dev = &hisi_hba->devices[device_id];
@@ -240,9 +241,9 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
}
if (need_lock) {
- spin_lock(&sas_dev->lock);
+ spin_lock_irqsave(&sas_dev->lock, flags);
list_del_init(&slot->entry);
- spin_unlock(&sas_dev->lock);
+ spin_unlock_irqrestore(&sas_dev->lock, flags);
} else {
list_del_init(&slot->entry);
}
@@ -403,6 +404,7 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
struct hisi_sas_cmd_hdr *cmd_hdr_base;
int dlvry_queue_slot, dlvry_queue;
struct sas_task *task = slot->task;
+ unsigned long flags;
int wr_q_index;
spin_lock(&dq->lock);
@@ -410,9 +412,9 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
list_add_tail(&slot->delivery, &dq->list);
spin_unlock(&dq->lock);
- spin_lock(&sas_dev->lock);
+ spin_lock_irqsave(&sas_dev->lock, flags);
list_add_tail(&slot->entry, &sas_dev->list);
- spin_unlock(&sas_dev->lock);
+ spin_unlock_irqrestore(&sas_dev->lock, flags);
dlvry_queue = dq->id;
dlvry_queue_slot = wr_q_index;
@@ -1103,12 +1105,13 @@ static void hisi_sas_release_task(struct hisi_hba *hisi_hba,
{
struct hisi_sas_slot *slot, *slot2;
struct hisi_sas_device *sas_dev = device->lldd_dev;
+ unsigned long flags;
- spin_lock(&sas_dev->lock);
+ spin_lock_irqsave(&sas_dev->lock, flags);
list_for_each_entry_safe(slot, slot2, &sas_dev->list, entry)
hisi_sas_do_release_task(hisi_hba, slot->task, slot, false);
- spin_unlock(&sas_dev->lock);
+ spin_unlock_irqrestore(&sas_dev->lock, flags);
}
void hisi_sas_release_tasks(struct hisi_hba *hisi_hba)
Hard interrupt cq_interrupt_v1_hw() could introduce double locks on &sas_dev->lock. <Deadlock #1> hisi_sas_abort_task_set() --> hisi_sas_release_task() --> spin_lock(&sas_dev->lock) <interrupt> --> cq_interrupt_v1_hw() --> hisi_sas_slot_task_free() --> spin_lock(&sas_dev->lock) <Deadlock #2> hisi_sas_abort_task() --> hisi_sas_softreset_ata_disk() --> hisi_sas_release_task() --> hisi_sas_do_release_task() --> hisi_sas_slot_task_free() --> spin_lock(&sas_dev->lock) <interrupt> --> cq_interrupt_v1_hw() --> hisi_sas_slot_task_free() --> spin_lock(&sas_dev->lock) <Deadlock #3> hisi_sas_queue_command() --> hisi_sas_task_deliver() --> spin_lock(&sas_dev->lock) <interrupt> --> cq_interrupt_v1_hw() --> hisi_sas_slot_task_free() --> spin_lock(&sas_dev->lock) This flaw was found by an experimental static analysis tool I am developing for irq-related deadlock. To prevent the potential deadlock, the patch use spin_lock_irqsave() on &sas_dev->lock. Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> --- drivers/scsi/hisi_sas/hisi_sas_main.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)