From c1dbec35ea5d94d4472876d79ce7e5b89de640a0 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 13 Apr 2019 22:47:22 +0000 Subject: [PATCH] qla2x00t-32gbit, target: Avoid that qlt_free_session_done() hangs Waiting for a long time inside a work item callback function is not a good idea. Split qlt_free_session_done() in two functions and call the second half asynchronously instead of waiting inside qlt_free_session_done(). git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@8184 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- qla2x00t-32gbit/qla2x00-target/README | 29 ----------------- qla2x00t-32gbit/qla_def.h | 4 +-- qla2x00t-32gbit/qla_init.c | 4 +-- qla2x00t-32gbit/qla_nvme.c | 6 ++-- qla2x00t-32gbit/qla_target.c | 46 ++++++++++++--------------- qla2x00t-32gbit/qla_target.h | 4 ++- 6 files changed, 29 insertions(+), 64 deletions(-) diff --git a/qla2x00t-32gbit/qla2x00-target/README b/qla2x00t-32gbit/qla2x00-target/README index 190a9846d..6c05debfc 100644 --- a/qla2x00t-32gbit/qla2x00-target/README +++ b/qla2x00t-32gbit/qla2x00-target/README @@ -68,32 +68,3 @@ Call Trace: [] ? retint_restore_args+0x13/0x43 [] ? kthread+0x0/0xc0 [] ? child_rip+0x0/0x30 - -When using RHEL 6 unloading this driver triggers a deadlock: - -INFO: task events/0:35 blocked for more than 120 seconds. - Not tainted 2.6.32-754.11.1.el6.x86_64.debug #1 -"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. -events/0 D ffff880077e24408 12408 35 2 0x00000000 - ffff88007e02fa60 0000000000000046 0000000000000000 7fffffffffffffff - 7fffffffffffffff 00000365a5588f80 ffff88007e028a80 ffff88000d7d93d8 - 0000000100346a5e 00000000000003c8 ffff88007e029040 ffff88007e02ffd8 -Call Trace: - [] schedule_timeout+0x265/0x330 - [] ? wait_for_common+0x4f/0x180 - [] ? wait_for_common+0x4f/0x180 - [] ? default_wake_function+0x0/0x20 - [] wait_for_common+0x12b/0x180 - [] ? default_wake_function+0x0/0x20 - [] wait_for_completion+0x1d/0x20 - [] ? qlt_free_session_done+0x1d6/0x930 [qla2xxx_scst] - [] ? qlt_free_session_done+0x0/0x930 [qla2xxx_scst] - [] ? worker_thread+0x21e/0x3f0 - [] ? worker_thread+0x1cb/0x3f0 - [] ? autoremove_wake_function+0x0/0x40 - [] ? worker_thread+0x0/0x3f0 - [] ? kthread+0xa0/0xc0 - [] ? child_rip+0x20/0x30 - [] ? retint_restore_args+0x13/0x43 - [] ? kthread+0x0/0xc0 - [] ? child_rip+0x0/0x30 diff --git a/qla2x00t-32gbit/qla_def.h b/qla2x00t-32gbit/qla_def.h index 0f984b3fb..b396d2dfe 100644 --- a/qla2x00t-32gbit/qla_def.h +++ b/qla2x00t-32gbit/qla_def.h @@ -2393,7 +2393,6 @@ typedef struct fc_port { #define NVME_FLAG_RESETTING 1 struct fc_port *conflict; - unsigned char logout_completed; int generation; struct se_session *se_sess; @@ -2401,8 +2400,9 @@ typedef struct fc_port { struct qla_tgt *tgt; unsigned long expires; struct list_head del_list_entry; - struct work_struct free_work; struct work_struct reg_work; + struct work_struct post_logout_work; + struct work_struct finish_logout_work; uint64_t jiffies_at_registration; struct qlt_plogi_ack_t *plogi_link[QLT_PLOGI_LINK_MAX]; struct completion *unreg_done; diff --git a/qla2x00t-32gbit/qla_init.c b/qla2x00t-32gbit/qla_init.c index d020aa2ae..cb1fde02d 100644 --- a/qla2x00t-32gbit/qla_init.c +++ b/qla2x00t-32gbit/qla_init.c @@ -224,7 +224,6 @@ qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport, goto done; fcport->flags |= FCF_ASYNC_SENT; - fcport->logout_completed = 0; fcport->disc_state = DSC_LOGIN_PEND; sp->type = SRB_LOGIN_CMD; @@ -1065,7 +1064,6 @@ qla24xx_async_prli(struct scsi_qla_host *vha, fc_port_t *fcport) return rval; fcport->flags |= FCF_ASYNC_SENT; - fcport->logout_completed = 0; sp->type = SRB_PRLI_CMD; sp->name = "prli"; @@ -4724,6 +4722,8 @@ qla2x00_alloc_fcport(scsi_qla_host_t *vha, gfp_t flags) INIT_WORK(&fcport->del_work, qla24xx_delete_sess_fn); INIT_WORK(&fcport->reg_work, qla_register_fcport_fn); + INIT_WORK(&fcport->post_logout_work, qlt_post_logout); + INIT_WORK(&fcport->finish_logout_work, qlt_finish_logout); INIT_LIST_HEAD(&fcport->gnl_entry); INIT_LIST_HEAD(&fcport->list); diff --git a/qla2x00t-32gbit/qla_nvme.c b/qla2x00t-32gbit/qla_nvme.c index 56e0d777f..7db0f73dc 100644 --- a/qla2x00t-32gbit/qla_nvme.c +++ b/qla2x00t-32gbit/qla_nvme.c @@ -564,10 +564,8 @@ static void qla_nvme_remoteport_delete(struct nvme_fc_remote_port *rport) } complete(&fcport->nvme_del_done); - if (!test_bit(UNLOADING, &fcport->vha->dpc_flags)) { - INIT_WORK(&fcport->free_work, qlt_free_session_done); - schedule_work(&fcport->free_work); - } + if (!test_bit(UNLOADING, &fcport->vha->dpc_flags)) + schedule_work(&fcport->post_logout_work); fcport->nvme_flag &= ~NVME_FLAG_DELETING; ql_log(ql_log_info, fcport->vha, 0x2110, diff --git a/qla2x00t-32gbit/qla_target.c b/qla2x00t-32gbit/qla_target.c index f0d00c864..08151cd92 100644 --- a/qla2x00t-32gbit/qla_target.c +++ b/qla2x00t-32gbit/qla_target.c @@ -698,7 +698,8 @@ void qla24xx_do_nack_work(struct scsi_qla_host *vha, struct qla_work_evt *e) case SRB_NACK_PRLI: t = e->u.nack.fcport; flush_work(&t->del_work); - flush_work(&t->free_work); + flush_work(&t->post_logout_work); + flush_work(&t->finish_logout_work); mutex_lock(&vha->vha_tgt.tgt_mutex); t = qlt_create_sess(vha, e->u.nack.fcport, 0); mutex_unlock(&vha->vha_tgt.tgt_mutex); @@ -969,16 +970,13 @@ qlt_send_first_logo(struct scsi_qla_host *vha, qlt_port_logo_t *logo) logo->cmd_count, res); } -void qlt_free_session_done(struct work_struct *work) +void qlt_post_logout(struct work_struct *work) { struct fc_port *sess = container_of(work, struct fc_port, - free_work); - struct qla_tgt *tgt = sess->tgt; + post_logout_work); struct scsi_qla_host *vha = sess->vha; struct qla_hw_data *ha = vha->hw; - unsigned long flags; bool logout_started = false; - scsi_qla_host_t *base_vha; struct qlt_plogi_ack_t *own = sess->plogi_link[QLT_PLOGI_LINK_SAME_WWN]; @@ -1036,22 +1034,19 @@ void qlt_free_session_done(struct work_struct *work) if (sess->se_sess != NULL) ha->tgt.tgt_ops->free_session(sess); - if (logout_started) { - bool traced = false; + if (!logout_started) + qlt_finish_logout(&sess->finish_logout_work); +} - while (!READ_ONCE(sess->logout_completed)) { - if (!traced) { - ql_dbg(ql_dbg_tgt_mgt, vha, 0xf086, - "%s: waiting for sess %p logout\n", - __func__, sess); - traced = true; - } - msleep(100); - } - - ql_dbg(ql_dbg_disc, vha, 0xf087, - "%s: sess %p logout completed\n", __func__, sess); - } +void qlt_finish_logout(struct work_struct *work) +{ + struct fc_port *sess = container_of(work, struct fc_port, + finish_logout_work); + struct qla_tgt *tgt = sess->tgt; + struct scsi_qla_host *vha = sess->vha; + struct qla_hw_data *ha = vha->hw; + scsi_qla_host_t *base_vha; + unsigned long flags; if (sess->logo_ack_needed) { sess->logo_ack_needed = 0; @@ -1088,7 +1083,8 @@ void qlt_free_session_done(struct work_struct *work) struct qlt_plogi_ack_t *con = sess->plogi_link[QLT_PLOGI_LINK_CONFLICT]; struct imm_ntfy_from_isp *iocb; - own = sess->plogi_link[QLT_PLOGI_LINK_SAME_WWN]; + struct qlt_plogi_ack_t *own = + sess->plogi_link[QLT_PLOGI_LINK_SAME_WWN]; if (con) { iocb = &con->iocb; @@ -1186,8 +1182,7 @@ void qlt_unreg_sess(struct fc_port *sess) sess->nvme_flag |= NVME_FLAG_DELETING; schedule_work(&sess->nvme_del_work); } else { - INIT_WORK(&sess->free_work, qlt_free_session_done); - schedule_work(&sess->free_work); + schedule_work(&sess->post_logout_work); } } EXPORT_SYMBOL(qlt_unreg_sess); @@ -1395,7 +1390,6 @@ static struct fc_port *qlt_create_sess( */ sess->logout_on_delete = 1; sess->keep_nport_handle = 0; - sess->logout_completed = 0; if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, &fcport->port_name[0], sess) < 0) { @@ -4734,7 +4728,7 @@ void qlt_logo_completion_handler(fc_port_t *fcport, int rc) fcport->d_id.b.al_pa, rc); } - fcport->logout_completed = 1; + schedule_work(&fcport->finish_logout_work); } /* diff --git a/qla2x00t-32gbit/qla_target.h b/qla2x00t-32gbit/qla_target.h index 1e8715a7a..fc38fb88c 100644 --- a/qla2x00t-32gbit/qla_target.h +++ b/qla2x00t-32gbit/qla_target.h @@ -1069,7 +1069,9 @@ extern void qlt_fc_port_deleted(struct scsi_qla_host *, fc_port_t *, int); extern int __init qlt_init(void); extern void qlt_exit(void); extern void qlt_update_vp_map(struct scsi_qla_host *, int); -extern void qlt_free_session_done(struct work_struct *); +extern void qlt_post_logout(struct work_struct *); +extern void qlt_finish_logout(struct work_struct *work); + /* * This macro is used during early initializations when host->active_mode * is not set. Right now, ha value is ignored.