Re: [PATCH 1/3] qla2xxx: Fixup locking for session deletion
On Thu, 22 Feb 2018, 12:49am, Hannes Reinecke wrote:
> Commit d8630bb95f46 ('Serialize session deletion by using work_lock') tries
> to fixup a deadlock when deleting sessions, but fails to take
> into account the locking rules. This patch resolves the situation
> by introducing a separate lock for processing the GNLIST response, and
> ensures that sess_lock is released before calling
> qlt_schedule_sess_delete().
>
> Cc: Himanshu Madhani
> Cc: Quinn Tran
> Fixes: d8630bb95f46 ("scsi: qla2xxx: Serialize session deletion by using
> work_lock")
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/qla2xxx/qla_def.h| 4 ++--
> drivers/scsi/qla2xxx/qla_init.c | 24 +++-
> drivers/scsi/qla2xxx/qla_os.c | 7 ++-
> drivers/scsi/qla2xxx/qla_target.c | 17 ++---
> 4 files changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index be7d6824581a..3ca4b6a5eddd 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -261,9 +261,9 @@
> struct name_list_extended {
> struct get_name_list_extended *l;
> dma_addr_t ldma;
> - struct list_headfcports;/* protect by sess_list */
> + struct list_headfcports;
> + spinlock_t fcports_lock;
> u32 size;
> - u8 sent;
> };
> /*
> * Timeout timer counts in seconds
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 590aa904fdef..4dd897c2c2b1 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -660,8 +660,7 @@ qla24xx_async_gnl_sp_done(void *s, int res)
> (loop_id & 0x7fff));
> }
>
> - spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
> - vha->gnl.sent = 0;
> + spin_lock_irqsave(&vha->gnl.fcports_lock, flags);
>
> INIT_LIST_HEAD(&h);
> fcport = tf = NULL;
> @@ -670,12 +669,16 @@ qla24xx_async_gnl_sp_done(void *s, int res)
>
> list_for_each_entry_safe(fcport, tf, &h, gnl_entry) {
> list_del_init(&fcport->gnl_entry);
> + spin_lock(&vha->hw->tgt.sess_lock);
> fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
> + spin_unlock(&vha->hw->tgt.sess_lock);
> ea.fcport = fcport;
>
> qla2x00_fcport_event_handler(vha, &ea);
> }
> + spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
>
> + spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
> /* create new fcport if fw has knowledge of new sessions */
> for (i = 0; i < n; i++) {
> port_id_t id;
> @@ -727,18 +730,21 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha,
> fc_port_t *fcport)
> ql_dbg(ql_dbg_disc, vha, 0x20d9,
> "Async-gnlist WWPN %8phC \n", fcport->port_name);
>
> - spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
> + spin_lock_irqsave(&vha->gnl.fcports_lock, flags);
> + if (!list_empty(&fcport->gnl_entry)) {
> + spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
> + rval = QLA_SUCCESS;
> + goto done;
> + }
> +
> + spin_lock(&vha->hw->tgt.sess_lock);
> fcport->disc_state = DSC_GNL;
> fcport->last_rscn_gen = fcport->rscn_gen;
> fcport->last_login_gen = fcport->login_gen;
> + spin_unlock(&vha->hw->tgt.sess_lock);
>
> list_add_tail(&fcport->gnl_entry, &vha->gnl.fcports);
> - if (vha->gnl.sent) {
> - spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
> - return QLA_SUCCESS;
> - }
> - vha->gnl.sent = 1;
> - spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
> + spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
>
> sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL);
> if (!sp)
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 12ee6e02d146..dc2b188fdce1 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -4575,6 +4575,7 @@ struct scsi_qla_host *qla2x00_create_host(struct
> scsi_host_template *sht,
>
> spin_lock_init(&vha->work_lock);
> spin_lock_init(&vha->cmd_list_lock);
> + spin_lock_init(&vha->gnl.fcports_lock);
> init_waitqueue_head(&vha->fcport_waitQ);
> init_waitqueue_head(&vha->vref_waitq);
>
> @@ -4875,6 +4876,8 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha,
> struct qla_work_evt *e)
> }
> qlt_plogi_ack_unref(vha, pla);
> } else {
> + fc_port_t *dfcp = NULL;
> +
> spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
> tfcp = qla2x00_find_fcport_by_nportid(vha,
> &e->u.new_sess.id, 1);
> @@ -4897,11 +4900,13 @@ void qla24xx_create_new_sess(struct scsi_qla
[PATCH 1/3] qla2xxx: Fixup locking for session deletion
Commit d8630bb95f46 ('Serialize session deletion by using work_lock') tries
to fixup a deadlock when deleting sessions, but fails to take
into account the locking rules. This patch resolves the situation
by introducing a separate lock for processing the GNLIST response, and
ensures that sess_lock is released before calling
qlt_schedule_sess_delete().
Cc: Himanshu Madhani
Cc: Quinn Tran
Fixes: d8630bb95f46 ("scsi: qla2xxx: Serialize session deletion by using
work_lock")
Signed-off-by: Hannes Reinecke
---
drivers/scsi/qla2xxx/qla_def.h| 4 ++--
drivers/scsi/qla2xxx/qla_init.c | 24 +++-
drivers/scsi/qla2xxx/qla_os.c | 7 ++-
drivers/scsi/qla2xxx/qla_target.c | 17 ++---
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index be7d6824581a..3ca4b6a5eddd 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -261,9 +261,9 @@
struct name_list_extended {
struct get_name_list_extended *l;
dma_addr_t ldma;
- struct list_headfcports;/* protect by sess_list */
+ struct list_headfcports;
+ spinlock_t fcports_lock;
u32 size;
- u8 sent;
};
/*
* Timeout timer counts in seconds
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 590aa904fdef..4dd897c2c2b1 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -660,8 +660,7 @@ qla24xx_async_gnl_sp_done(void *s, int res)
(loop_id & 0x7fff));
}
- spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
- vha->gnl.sent = 0;
+ spin_lock_irqsave(&vha->gnl.fcports_lock, flags);
INIT_LIST_HEAD(&h);
fcport = tf = NULL;
@@ -670,12 +669,16 @@ qla24xx_async_gnl_sp_done(void *s, int res)
list_for_each_entry_safe(fcport, tf, &h, gnl_entry) {
list_del_init(&fcport->gnl_entry);
+ spin_lock(&vha->hw->tgt.sess_lock);
fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
+ spin_unlock(&vha->hw->tgt.sess_lock);
ea.fcport = fcport;
qla2x00_fcport_event_handler(vha, &ea);
}
+ spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
+ spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
/* create new fcport if fw has knowledge of new sessions */
for (i = 0; i < n; i++) {
port_id_t id;
@@ -727,18 +730,21 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha,
fc_port_t *fcport)
ql_dbg(ql_dbg_disc, vha, 0x20d9,
"Async-gnlist WWPN %8phC \n", fcport->port_name);
- spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
+ spin_lock_irqsave(&vha->gnl.fcports_lock, flags);
+ if (!list_empty(&fcport->gnl_entry)) {
+ spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
+ rval = QLA_SUCCESS;
+ goto done;
+ }
+
+ spin_lock(&vha->hw->tgt.sess_lock);
fcport->disc_state = DSC_GNL;
fcport->last_rscn_gen = fcport->rscn_gen;
fcport->last_login_gen = fcport->login_gen;
+ spin_unlock(&vha->hw->tgt.sess_lock);
list_add_tail(&fcport->gnl_entry, &vha->gnl.fcports);
- if (vha->gnl.sent) {
- spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
- return QLA_SUCCESS;
- }
- vha->gnl.sent = 1;
- spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
+ spin_unlock_irqrestore(&vha->gnl.fcports_lock, flags);
sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL);
if (!sp)
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 12ee6e02d146..dc2b188fdce1 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4575,6 +4575,7 @@ struct scsi_qla_host *qla2x00_create_host(struct
scsi_host_template *sht,
spin_lock_init(&vha->work_lock);
spin_lock_init(&vha->cmd_list_lock);
+ spin_lock_init(&vha->gnl.fcports_lock);
init_waitqueue_head(&vha->fcport_waitQ);
init_waitqueue_head(&vha->vref_waitq);
@@ -4875,6 +4876,8 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha,
struct qla_work_evt *e)
}
qlt_plogi_ack_unref(vha, pla);
} else {
+ fc_port_t *dfcp = NULL;
+
spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
tfcp = qla2x00_find_fcport_by_nportid(vha,
&e->u.new_sess.id, 1);
@@ -4897,11 +4900,13 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha,
struct qla_work_evt *e)
default:
fcport->login_pause = 1;
