When taking the board offline while performing i/o, unsafe locking errors occurred and irq level isn't properly managed. In lpfc_sli_hba_down, spin_lock_irqsave(&phba->hbalock, flags) does not disable softirqs raised from timer expiry. It is possible that a softirq is raised from the lpfc_els_retry_delay routine and recursively requests the same phba->hbalock spinlock causing deadlock. Address the deadlocks by creating a new port_list lock. The softirq behavior can then be managed a level deeper into the calling sequences. Signed-off-by: Dick Kennedy <dick.kennedy@xxxxxxxxxxxx> Signed-off-by: James Smart <james.smart@xxxxxxxxxxxx> --- drivers/scsi/lpfc/lpfc.h | 1 + drivers/scsi/lpfc/lpfc_ct.c | 6 +++--- drivers/scsi/lpfc/lpfc_els.c | 3 +++ drivers/scsi/lpfc/lpfc_hbadisc.c | 6 +++--- drivers/scsi/lpfc/lpfc_init.c | 19 +++++++++++-------- drivers/scsi/lpfc/lpfc_sli.c | 25 ++++++++++++++++++++++++- drivers/scsi/lpfc/lpfc_vport.c | 14 +++++++------- 7 files changed, 52 insertions(+), 22 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h index 322b24f826a4..327751447355 100644 --- a/drivers/scsi/lpfc/lpfc.h +++ b/drivers/scsi/lpfc/lpfc.h @@ -964,6 +964,7 @@ struct lpfc_hba { uint32_t intr_mode; #define LPFC_INTR_ERROR 0xFFFFFFFF struct list_head port_list; + spinlock_t port_list_lock; /* lock for port_list mutations */ struct lpfc_vport *pport; /* physical lpfc_vport pointer */ uint16_t max_vpi; /* Maximum virtual nports */ #define LPFC_MAX_VPI 0xFFFF /* Max number of VPI supported */ diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c index 0eae8051e920..789ad1502534 100644 --- a/drivers/scsi/lpfc/lpfc_ct.c +++ b/drivers/scsi/lpfc/lpfc_ct.c @@ -445,14 +445,14 @@ lpfc_find_vport_by_did(struct lpfc_hba *phba, uint32_t did) { struct lpfc_vport *vport_curr; unsigned long flags; - spin_lock_irqsave(&phba->hbalock, flags); + spin_lock_irqsave(&phba->port_list_lock, flags); list_for_each_entry(vport_curr, &phba->port_list, listentry) { if ((vport_curr->fc_myDID) && (vport_curr->fc_myDID == did)) { - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return vport_curr; } } - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return NULL; } diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c index 4dda969e947c..f1c1faa74b46 100644 --- a/drivers/scsi/lpfc/lpfc_els.c +++ b/drivers/scsi/lpfc/lpfc_els.c @@ -7673,8 +7673,11 @@ void lpfc_els_flush_all_cmd(struct lpfc_hba *phba) { struct lpfc_vport *vport; + + spin_lock_irq(&phba->port_list_lock); list_for_each_entry(vport, &phba->port_list, listentry) lpfc_els_flush_cmd(vport); + spin_unlock_irq(&phba->port_list_lock); return; } diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c index 235abd50e530..f9a038ec5256 100644 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c @@ -5938,14 +5938,14 @@ lpfc_find_vport_by_vpid(struct lpfc_hba *phba, uint16_t vpi) } } - spin_lock_irqsave(&phba->hbalock, flags); + spin_lock_irqsave(&phba->port_list_lock, flags); list_for_each_entry(vport, &phba->port_list, listentry) { if (vport->vpi == i) { - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return vport; } } - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return NULL; } diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index 90fb83f88179..bb2bff7b56b4 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -3988,9 +3988,9 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, struct device *dev) if (error) goto out_put_shost; - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_add_tail(&vport->listentry, &phba->port_list); - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); return vport; out_put_shost: @@ -4016,9 +4016,9 @@ destroy_port(struct lpfc_vport *vport) fc_remove_host(shost); scsi_remove_host(shost); - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_del_init(&vport->listentry); - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); lpfc_cleanup(vport); return; @@ -5621,7 +5621,10 @@ lpfc_setup_driver_resource_phase1(struct lpfc_hba *phba) /* Initialize ndlp management spinlock */ spin_lock_init(&phba->ndlp_lock); + /* Initialize port_list spinlock */ + spin_lock_init(&phba->port_list_lock); INIT_LIST_HEAD(&phba->port_list); + INIT_LIST_HEAD(&phba->work_list); init_waitqueue_head(&phba->wait_4_mlo_m_q); @@ -10985,9 +10988,9 @@ lpfc_pci_remove_one_s3(struct pci_dev *pdev) kfree(phba->vpi_ids); lpfc_stop_hba_timers(phba); - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_del_init(&vport->listentry); - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); lpfc_debugfs_terminate(vport); @@ -11797,9 +11800,9 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev) lpfc_sli4_hba_unset(phba); lpfc_stop_hba_timers(phba); - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_del_init(&vport->listentry); - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); /* Perform scsi free before driver resource_unset since scsi * buffers are released to their corresponding pools here. diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c index a95c823cd1a4..495de99ed82d 100644 --- a/drivers/scsi/lpfc/lpfc_sli.c +++ b/drivers/scsi/lpfc/lpfc_sli.c @@ -10273,8 +10273,12 @@ lpfc_sli_mbox_sys_flush(struct lpfc_hba *phba) LPFC_MBOXQ_t *pmb; unsigned long iflag; + /* Disable softirqs, including timers from obtaining phba->hbalock */ + local_bh_disable(); + /* Flush all the mailbox commands in the mbox system */ spin_lock_irqsave(&phba->hbalock, iflag); + /* The pending mailbox command queue */ list_splice_init(&phba->sli.mboxq, &completions); /* The outstanding active mailbox command */ @@ -10287,6 +10291,9 @@ lpfc_sli_mbox_sys_flush(struct lpfc_hba *phba) list_splice_init(&phba->sli.mboxq_cmpl, &completions); spin_unlock_irqrestore(&phba->hbalock, iflag); + /* Enable softirqs again, done with phba->hbalock */ + local_bh_enable(); + /* Return all flushed mailbox commands with MBX_NOT_FINISHED status */ while (!list_empty(&completions)) { list_remove_head(&completions, pmb, LPFC_MBOXQ_t, list); @@ -10426,6 +10433,9 @@ lpfc_sli_hba_down(struct lpfc_hba *phba) lpfc_hba_down_prep(phba); + /* Disable softirqs, including timers from obtaining phba->hbalock */ + local_bh_disable(); + lpfc_fabric_abort_hba(phba); spin_lock_irqsave(&phba->hbalock, flags); @@ -10479,6 +10489,9 @@ lpfc_sli_hba_down(struct lpfc_hba *phba) kfree(buf_ptr); } + /* Enable softirqs again, done with phba->hbalock */ + local_bh_enable(); + /* Return any active mbox cmds */ del_timer_sync(&psli->mbox_tmo); @@ -11782,6 +11795,9 @@ lpfc_sli_mbox_sys_shutdown(struct lpfc_hba *phba, int mbx_action) } timeout = msecs_to_jiffies(LPFC_MBOX_TMO * 1000) + jiffies; + /* Disable softirqs, including timers from obtaining phba->hbalock */ + local_bh_disable(); + spin_lock_irq(&phba->hbalock); psli->sli_flag |= LPFC_SLI_ASYNC_MBX_BLK; @@ -11795,6 +11811,9 @@ lpfc_sli_mbox_sys_shutdown(struct lpfc_hba *phba, int mbx_action) 1000) + jiffies; spin_unlock_irq(&phba->hbalock); + /* Enable softirqs again, done with phba->hbalock */ + local_bh_enable(); + while (phba->sli.mbox_active) { /* Check active mailbox complete status every 2ms */ msleep(2); @@ -11804,9 +11823,13 @@ lpfc_sli_mbox_sys_shutdown(struct lpfc_hba *phba, int mbx_action) */ break; } - } else + } else { spin_unlock_irq(&phba->hbalock); + /* Enable softirqs again, done with phba->hbalock */ + local_bh_enable(); + } + lpfc_sli_mbox_sys_flush(phba); } diff --git a/drivers/scsi/lpfc/lpfc_vport.c b/drivers/scsi/lpfc/lpfc_vport.c index 1ff0f7de9105..c340e0e47473 100644 --- a/drivers/scsi/lpfc/lpfc_vport.c +++ b/drivers/scsi/lpfc/lpfc_vport.c @@ -207,7 +207,7 @@ lpfc_unique_wwpn(struct lpfc_hba *phba, struct lpfc_vport *new_vport) struct lpfc_vport *vport; unsigned long flags; - spin_lock_irqsave(&phba->hbalock, flags); + spin_lock_irqsave(&phba->port_list_lock, flags); list_for_each_entry(vport, &phba->port_list, listentry) { if (vport == new_vport) continue; @@ -215,11 +215,11 @@ lpfc_unique_wwpn(struct lpfc_hba *phba, struct lpfc_vport *new_vport) if (memcmp(&vport->fc_sparam.portName, &new_vport->fc_sparam.portName, sizeof(struct lpfc_name)) == 0) { - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return 0; } } - spin_unlock_irqrestore(&phba->hbalock, flags); + spin_unlock_irqrestore(&phba->port_list_lock, flags); return 1; } @@ -825,9 +825,9 @@ lpfc_vport_delete(struct fc_vport *fc_vport) lpfc_free_vpi(phba, vport->vpi); vport->work_port_events = 0; - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_del_init(&vport->listentry); - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); lpfc_printf_vlog(vport, KERN_ERR, LOG_VPORT, "1828 Vport Deleted.\n"); scsi_host_put(shost); @@ -844,7 +844,7 @@ lpfc_create_vport_work_array(struct lpfc_hba *phba) GFP_KERNEL); if (vports == NULL) return NULL; - spin_lock_irq(&phba->hbalock); + spin_lock_irq(&phba->port_list_lock); list_for_each_entry(port_iterator, &phba->port_list, listentry) { if (port_iterator->load_flag & FC_UNLOADING) continue; @@ -856,7 +856,7 @@ lpfc_create_vport_work_array(struct lpfc_hba *phba) } vports[index++] = port_iterator; } - spin_unlock_irq(&phba->hbalock); + spin_unlock_irq(&phba->port_list_lock); return vports; } -- 2.13.1