Skip to content

Commit 300ec74

Browse files
bvanasschemartinkpetersen
bvanassche
authored and
martinkpetersen
committed
scsi: qla2xxx: Fix hardirq-unsafe locking
Since fc_remote_port_delete() must be called with interrupts enabled, do not disable interrupts when calling that function. Remove the lockin calls from around the put_sess() call. This is safe because the function that is called when the final reference is dropped, qlt_unreg_sess(), grabs the proper locks. This patch avoids that lockdep reports the following: WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected kworker/2:1/62 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire: 0000000009e679b3 (&(&k->k_lock)->rlock){+.+.}, at: klist_next+0x43/0x1d0 and this task is already holding: 00000000a033b71c (&(&ha->tgt.sess_lock)->rlock){-...}, at: qla24xx_delete_sess_fn+0x55/0xf0 [qla2xxx_scst] which would create a new lock dependency: (&(&ha->tgt.sess_lock)->rlock){-...} -> (&(&k->k_lock)->rlock){+.+.} but this new dependency connects a HARDIRQ-irq-safe lock: (&(&ha->tgt.sess_lock)->rlock){-...} ... which became HARDIRQ-irq-safe at: lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 qla24xx_report_id_acquisition+0xa69/0xe30 [qla2xxx_scst] qla24xx_process_response_queue+0x69e/0x1270 [qla2xxx_scst] qla24xx_msix_rsp_q+0x79/0xf0 [qla2xxx_scst] __handle_irq_event_percpu+0x79/0x3c0 handle_irq_event_percpu+0x70/0xf0 handle_irq_event+0x5a/0x8b handle_edge_irq+0x12c/0x310 handle_irq+0x192/0x20a do_IRQ+0x73/0x160 ret_from_intr+0x0/0x1d default_idle+0x23/0x1f0 arch_cpu_idle+0x15/0x20 default_idle_call+0x35/0x40 do_idle+0x2bb/0x2e0 cpu_startup_entry+0x1d/0x20 start_secondary+0x2a8/0x320 secondary_startup_64+0xa4/0xb0 to a HARDIRQ-irq-unsafe lock: (&(&k->k_lock)->rlock){+.+.} ... which became HARDIRQ-irq-unsafe at: ... lock_acquire+0xe3/0x200 _raw_spin_lock+0x32/0x50 klist_add_tail+0x33/0xb0 device_add+0x7e1/0xb50 device_create_groups_vargs+0x11c/0x150 device_create_with_groups+0x89/0xb0 vtconsole_class_init+0xb2/0x124 do_one_initcall+0xc5/0x3ce kernel_init_freeable+0x295/0x32e kernel_init+0x11/0x11b ret_from_fork+0x3a/0x50 other info that might help us debug this: Possible interrupt unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&(&k->k_lock)->rlock); local_irq_disable(); lock(&(&ha->tgt.sess_lock)->rlock); lock(&(&k->k_lock)->rlock); <Interrupt> lock(&(&ha->tgt.sess_lock)->rlock); *** DEADLOCK *** 3 locks held by kworker/2:1/62: #0: 00000000a4319c16 ((wq_completion)"qla2xxx_wq"){+.+.}, at: process_one_work+0x437/0xa80 #1: 00000000ffa34c42 ((work_completion)(&sess->del_work)){+.+.}, at: process_one_work+0x437/0xa80 #2: 00000000a033b71c (&(&ha->tgt.sess_lock)->rlock){-...}, at: qla24xx_delete_sess_fn+0x55/0xf0 [qla2xxx_scst] the dependencies between HARDIRQ-irq-safe lock and the holding lock: -> (&(&ha->tgt.sess_lock)->rlock){-...} ops: 8 { IN-HARDIRQ-W at: lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 qla24xx_report_id_acquisition+0xa69/0xe30 [qla2xxx_scst] qla24xx_process_response_queue+0x69e/0x1270 [qla2xxx_scst] qla24xx_msix_rsp_q+0x79/0xf0 [qla2xxx_scst] __handle_irq_event_percpu+0x79/0x3c0 handle_irq_event_percpu+0x70/0xf0 handle_irq_event+0x5a/0x8b handle_edge_irq+0x12c/0x310 handle_irq+0x192/0x20a do_IRQ+0x73/0x160 ret_from_intr+0x0/0x1d default_idle+0x23/0x1f0 arch_cpu_idle+0x15/0x20 default_idle_call+0x35/0x40 do_idle+0x2bb/0x2e0 cpu_startup_entry+0x1d/0x20 start_secondary+0x2a8/0x320 secondary_startup_64+0xa4/0xb0 INITIAL USE at: lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 qla24xx_report_id_acquisition+0xa69/0xe30 [qla2xxx_scst] qla24xx_process_response_queue+0x69e/0x1270 [qla2xxx_scst] qla24xx_msix_rsp_q+0x79/0xf0 [qla2xxx_scst] __handle_irq_event_percpu+0x79/0x3c0 handle_irq_event_percpu+0x70/0xf0 handle_irq_event+0x5a/0x8b handle_edge_irq+0x12c/0x310 handle_irq+0x192/0x20a do_IRQ+0x73/0x160 ret_from_intr+0x0/0x1d default_idle+0x23/0x1f0 arch_cpu_idle+0x15/0x20 default_idle_call+0x35/0x40 do_idle+0x2bb/0x2e0 cpu_startup_entry+0x1d/0x20 start_secondary+0x2a8/0x320 secondary_startup_64+0xa4/0xb0 } ... key at: [<ffffffffa0c0d080>] __key.85462+0x0/0xfffffffffff7df80 [qla2xxx_scst] ... acquired at: lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 klist_next+0x43/0x1d0 device_for_each_child+0x96/0x110 scsi_target_block+0x3c/0x40 [scsi_mod] fc_remote_port_delete+0xe7/0x1c0 [scsi_transport_fc] qla2x00_mark_device_lost+0xa0b/0xa30 [qla2xxx_scst] qlt_unreg_sess+0x1c6/0x380 [qla2xxx_scst] qla24xx_delete_sess_fn+0xe6/0xf0 [qla2xxx_scst] process_one_work+0x511/0xa80 worker_thread+0x67/0x5b0 kthread+0x1d2/0x1f0 ret_from_fork+0x3a/0x50 the dependencies between the lock to be acquired and HARDIRQ-irq-unsafe lock: -> (&(&k->k_lock)->rlock){+.+.} ops: 13831 { HARDIRQ-ON-W at: lock_acquire+0xe3/0x200 _raw_spin_lock+0x32/0x50 klist_add_tail+0x33/0xb0 device_add+0x7e1/0xb50 device_create_groups_vargs+0x11c/0x150 device_create_with_groups+0x89/0xb0 vtconsole_class_init+0xb2/0x124 do_one_initcall+0xc5/0x3ce kernel_init_freeable+0x295/0x32e kernel_init+0x11/0x11b ret_from_fork+0x3a/0x50 SOFTIRQ-ON-W at: lock_acquire+0xe3/0x200 _raw_spin_lock+0x32/0x50 klist_add_tail+0x33/0xb0 device_add+0x7e1/0xb50 device_create_groups_vargs+0x11c/0x150 device_create_with_groups+0x89/0xb0 vtconsole_class_init+0xb2/0x124 do_one_initcall+0xc5/0x3ce kernel_init_freeable+0x295/0x32e kernel_init+0x11/0x11b ret_from_fork+0x3a/0x50 INITIAL USE at: lock_acquire+0xe3/0x200 _raw_spin_lock+0x32/0x50 klist_add_tail+0x33/0xb0 device_add+0x7e1/0xb50 device_create_groups_vargs+0x11c/0x150 device_create_with_groups+0x89/0xb0 vtconsole_class_init+0xb2/0x124 do_one_initcall+0xc5/0x3ce kernel_init_freeable+0x295/0x32e kernel_init+0x11/0x11b ret_from_fork+0x3a/0x50 } ... key at: [<ffffffff83ed8780>] __key.15491+0x0/0x40 ... acquired at: lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 klist_next+0x43/0x1d0 device_for_each_child+0x96/0x110 scsi_target_block+0x3c/0x40 [scsi_mod] fc_remote_port_delete+0xe7/0x1c0 [scsi_transport_fc] qla2x00_mark_device_lost+0xa0b/0xa30 [qla2xxx_scst] qlt_unreg_sess+0x1c6/0x380 [qla2xxx_scst] qla24xx_delete_sess_fn+0xe6/0xf0 [qla2xxx_scst] process_one_work+0x511/0xa80 worker_thread+0x67/0x5b0 kthread+0x1d2/0x1f0 ret_from_fork+0x3a/0x50 stack backtrace: CPU: 2 PID: 62 Comm: kworker/2:1 Tainted: G O 5.0.7-dbg+ #8 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 Workqueue: qla2xxx_wq qla24xx_delete_sess_fn [qla2xxx_scst] Call Trace: dump_stack+0x86/0xca check_usage.cold.52+0x473/0x563 __lock_acquire+0x11c0/0x23e0 lock_acquire+0xe3/0x200 _raw_spin_lock_irqsave+0x3d/0x60 klist_next+0x43/0x1d0 device_for_each_child+0x96/0x110 scsi_target_block+0x3c/0x40 [scsi_mod] fc_remote_port_delete+0xe7/0x1c0 [scsi_transport_fc] qla2x00_mark_device_lost+0xa0b/0xa30 [qla2xxx_scst] qlt_unreg_sess+0x1c6/0x380 [qla2xxx_scst] qla24xx_delete_sess_fn+0xe6/0xf0 [qla2xxx_scst] process_one_work+0x511/0xa80 worker_thread+0x67/0x5b0 kthread+0x1d2/0x1f0 ret_from_fork+0x3a/0x50 Cc: Himanshu Madhani <hmadhani@marvell.com> Cc: Giridhar Malavali <gmalavali@marvell.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> Acked-by: Himanshu Madhani <hmadhani@marvell.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent db4bf82 commit 300ec74

File tree

2 files changed

+8
-19
lines changed

2 files changed

+8
-19
lines changed

‎drivers/scsi/qla2xxx/qla_target.c

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,6 @@ int qla24xx_async_notify_ack(scsi_qla_host_t *vha, fc_port_t *fcport,
682682
void qla24xx_do_nack_work(struct scsi_qla_host *vha, struct qla_work_evt *e)
683683
{
684684
fc_port_t *t;
685-
unsigned long flags;
686685

687686
switch (e->u.nack.type) {
688687
case SRB_NACK_PRLI:
@@ -695,10 +694,8 @@ void qla24xx_do_nack_work(struct scsi_qla_host *vha, struct qla_work_evt *e)
695694
if (t) {
696695
ql_log(ql_log_info, vha, 0xd034,
697696
"%s create sess success %p", __func__, t);
698-
spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
699697
/* create sess has an extra kref */
700698
vha->hw->tgt.tgt_ops->put_sess(e->u.nack.fcport);
701-
spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
702699
}
703700
break;
704701
}
@@ -710,17 +707,13 @@ void qla24xx_delete_sess_fn(struct work_struct *work)
710707
{
711708
fc_port_t *fcport = container_of(work, struct fc_port, del_work);
712709
struct qla_hw_data *ha = fcport->vha->hw;
713-
unsigned long flags;
714-
715-
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
716710

717711
if (fcport->se_sess) {
718712
ha->tgt.tgt_ops->shutdown_sess(fcport);
719713
ha->tgt.tgt_ops->put_sess(fcport);
720714
} else {
721715
qlt_unreg_sess(fcport);
722716
}
723-
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
724717
}
725718

726719
/*
@@ -789,8 +782,9 @@ void qlt_fc_port_added(struct scsi_qla_host *vha, fc_port_t *fcport)
789782
fcport->port_name, sess->loop_id);
790783
sess->local = 0;
791784
}
792-
ha->tgt.tgt_ops->put_sess(sess);
793785
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
786+
787+
ha->tgt.tgt_ops->put_sess(sess);
794788
}
795789

796790
/*
@@ -4175,9 +4169,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd)
41754169
/*
41764170
* Drop extra session reference from qlt_handle_cmd_for_atio().
41774171
*/
4178-
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
41794172
ha->tgt.tgt_ops->put_sess(sess);
4180-
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
41814173
return;
41824174

41834175
out_term:
@@ -4194,9 +4186,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd)
41944186
target_free_tag(sess->se_sess, &cmd->se_cmd);
41954187
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
41964188

4197-
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
41984189
ha->tgt.tgt_ops->put_sess(sess);
4199-
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
42004190
}
42014191

42024192
static void qlt_do_work(struct work_struct *work)
@@ -4405,9 +4395,7 @@ static int qlt_handle_cmd_for_atio(struct scsi_qla_host *vha,
44054395
if (!cmd) {
44064396
ql_dbg(ql_dbg_io, vha, 0x3062,
44074397
"qla_target(%d): Allocation of cmd failed\n", vha->vp_idx);
4408-
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
44094398
ha->tgt.tgt_ops->put_sess(sess);
4410-
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
44114399
return -EBUSY;
44124400
}
44134401

@@ -6258,17 +6246,19 @@ static void qlt_abort_work(struct qla_tgt *tgt,
62586246
}
62596247

62606248
rc = __qlt_24xx_handle_abts(vha, &prm->abts, sess);
6261-
ha->tgt.tgt_ops->put_sess(sess);
62626249
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
62636250

6251+
ha->tgt.tgt_ops->put_sess(sess);
6252+
62646253
if (rc != 0)
62656254
goto out_term;
62666255
return;
62676256

62686257
out_term2:
6258+
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
6259+
62696260
if (sess)
62706261
ha->tgt.tgt_ops->put_sess(sess);
6271-
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
62726262

62736263
out_term:
62746264
spin_lock_irqsave(&ha->hardware_lock, flags);
@@ -6326,9 +6316,10 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
63266316
scsilun_to_int((struct scsi_lun *)&a->u.isp24.fcp_cmnd.lun);
63276317

63286318
rc = qlt_issue_task_mgmt(sess, unpacked_lun, fn, iocb, 0);
6329-
ha->tgt.tgt_ops->put_sess(sess);
63306319
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
63316320

6321+
ha->tgt.tgt_ops->put_sess(sess);
6322+
63326323
if (rc != 0)
63336324
goto out_term;
63346325
return;

‎drivers/scsi/qla2xxx/tcm_qla2xxx.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,6 @@ static void tcm_qla2xxx_put_sess(struct fc_port *sess)
340340
if (!sess)
341341
return;
342342

343-
assert_spin_locked(&sess->vha->hw->tgt.sess_lock);
344343
kref_put(&sess->sess_kref, tcm_qla2xxx_release_session);
345344
}
346345

@@ -798,7 +797,6 @@ static void tcm_qla2xxx_clear_nacl_from_fcport_map(struct fc_port *sess)
798797

799798
static void tcm_qla2xxx_shutdown_sess(struct fc_port *sess)
800799
{
801-
assert_spin_locked(&sess->vha->hw->tgt.sess_lock);
802800
target_sess_cmd_list_set_waiting(sess->se_sess);
803801
}
804802

0 commit comments

Comments
 (0)