Hi Quinn, thanks for the explanation - seems like the put function is indeed a duplicate of the target core one, I should have noticed that. What do you think about the patch below, which just adds a local se_kref for the qla2xxx internal usage. It's initialized to 1 and gets an additional put when ->close_session is called. With that this series will work for qla2xxx with a few additional changes. The full series with qla2xxx support is also available here: git://git.infradead.org/users/hch/scsi.git target-session-cleanup gitweb: http://git.infradead.org/users/hch/scsi.git/shortlog/refs/heads/target-session-cleanup --- >From c7d648c9471e18b06b4eb6ef840ca2a3004cb3dd Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@xxxxxx> Date: Thu, 14 Jan 2016 18:47:42 +0100 Subject: tcm_qla2xxx: introduce a private sess_kref This stops abusing the common sess_kref to overload it for private usage. Besides making the code cleaner this prepares for removing the common sess_kref. Signed-off-by: Christoph Hellwig <hch@xxxxxx> --- drivers/scsi/qla2xxx/qla_target.c | 51 ++++++++++++++++++++++---------------- drivers/scsi/qla2xxx/qla_target.h | 4 +-- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 20 +-------------- 3 files changed, 33 insertions(+), 42 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 80ae367..8fd8f9f 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -637,8 +637,10 @@ static void qlt_free_session_done(struct work_struct *work) } /* ha->tgt.sess_lock supposed to be held on entry */ -void qlt_unreg_sess(struct qla_tgt_sess *sess) +static void qlt_release_session(struct kref *kref) { + struct qla_tgt_sess *sess = + container_of(kref, struct qla_tgt_sess, sess_kref); struct scsi_qla_host *vha = sess->vha; vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); @@ -650,8 +652,16 @@ void qlt_unreg_sess(struct qla_tgt_sess *sess) INIT_WORK(&sess->free_work, qlt_free_session_done); schedule_work(&sess->free_work); } -EXPORT_SYMBOL(qlt_unreg_sess); +void qlt_put_sess(struct qla_tgt_sess *sess) +{ + if (!sess) + return; + + assert_spin_locked(&sess->vha->hw->tgt.sess_lock); + kref_put(&sess->sess_kref, qlt_release_session); +} +EXPORT_SYMBOL(qlt_put_sess); static int qlt_reset(struct scsi_qla_host *vha, void *iocb, int mcmd) { @@ -857,7 +867,7 @@ static void qlt_del_sess_work_fn(struct delayed_work *work) "Timeout: sess %p about to be deleted\n", sess); ha->tgt.tgt_ops->shutdown_sess(sess); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); } else { schedule_delayed_work(&tgt->sess_del_work, sess->expires - elapsed); @@ -904,7 +914,7 @@ static struct qla_tgt_sess *qlt_create_sess( if (sess->deleted) qlt_undelete_sess(sess); - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); ha->tgt.tgt_ops->update_sess(sess, fcport->d_id, fcport->loop_id, (fcport->flags & FCF_CONF_COMP_SUPPORTED)); @@ -934,6 +944,7 @@ static struct qla_tgt_sess *qlt_create_sess( sess->s_id = fcport->d_id; sess->loop_id = fcport->loop_id; sess->local = local; + kref_init(&sess->sess_kref); INIT_LIST_HEAD(&sess->del_list_entry); /* Under normal circumstances we want to logout from firmware when @@ -962,7 +973,7 @@ static struct qla_tgt_sess *qlt_create_sess( * Take an extra reference to ->sess_kref here to handle qla_tgt_sess * access across ->tgt.sess_lock reaquire. */ - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); sess->conf_compl_supported = (fcport->flags & FCF_CONF_COMP_SUPPORTED); BUILD_BUG_ON(sizeof(sess->port_name) != sizeof(fcport->port_name)); @@ -1022,7 +1033,7 @@ void qlt_fc_port_added(struct scsi_qla_host *vha, fc_port_t *fcport) spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); return; } else { - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); if (sess->deleted) { qlt_undelete_sess(sess); @@ -1047,7 +1058,7 @@ void qlt_fc_port_added(struct scsi_qla_host *vha, fc_port_t *fcport) fcport->port_name, sess->loop_id); sess->local = 0; } - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); } @@ -3780,7 +3791,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd) * Drop extra session reference from qla_tgt_handle_cmd_for_atio*( */ spin_lock_irqsave(&ha->tgt.sess_lock, flags); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); return; @@ -3799,7 +3810,7 @@ out_term: spin_unlock_irqrestore(&ha->hardware_lock, flags); spin_lock_irqsave(&ha->tgt.sess_lock, flags); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); } @@ -3899,13 +3910,13 @@ static void qlt_create_sess_from_atio(struct work_struct *work) if (!cmd) { spin_lock_irqsave(&ha->hardware_lock, flags); qlt_send_busy(vha, &op->atio, SAM_STAT_BUSY); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->hardware_lock, flags); kfree(op); return; } /* - * __qlt_do_work() will call ha->tgt.tgt_ops->put_sess() to release + * __qlt_do_work() will call qlt_put_sess() to release * the extra reference taken above by qlt_make_local_sess() */ __qlt_do_work(cmd); @@ -3966,13 +3977,13 @@ static int qlt_handle_cmd_for_atio(struct scsi_qla_host *vha, /* * Do kref_get() before returning + dropping qla_hw_data->hardware_lock. */ - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); cmd = qlt_get_tag(vha, sess, atio); if (!cmd) { ql_dbg(ql_dbg_io, vha, 0x3062, "qla_target(%d): Allocation of cmd failed\n", vha->vp_idx); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); return -ENOMEM; } @@ -5874,7 +5885,7 @@ static void qlt_abort_work(struct qla_tgt *tgt, goto out_term2; } - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); } spin_lock_irqsave(&ha->hardware_lock, flags); @@ -5887,7 +5898,7 @@ static void qlt_abort_work(struct qla_tgt *tgt, goto out_term; spin_unlock_irqrestore(&ha->hardware_lock, flags); - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2); return; @@ -5898,8 +5909,7 @@ out_term: qlt_24xx_send_abts_resp(vha, &prm->abts, FCP_TMF_REJECTED, false); spin_unlock_irqrestore(&ha->hardware_lock, flags); - if (sess) - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2); } @@ -5939,7 +5949,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt, goto out_term; } - kref_get(&sess->se_sess->sess_kref); + kref_get(&sess->sess_kref); } iocb = a; @@ -5951,14 +5961,13 @@ static void qlt_tmr_work(struct qla_tgt *tgt, if (rc != 0) goto out_term; - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); return; out_term: qlt_send_term_exchange(vha, NULL, &prm->tm_iocb2, 0); - if (sess) - ha->tgt.tgt_ops->put_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); } diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h index 409bd5f..75f1c24 100644 --- a/drivers/scsi/qla2xxx/qla_target.h +++ b/drivers/scsi/qla2xxx/qla_target.h @@ -738,7 +738,6 @@ struct qla_tgt_func_tmpl { struct qla_tgt_sess *(*find_sess_by_s_id)(struct scsi_qla_host *, const uint8_t *); void (*clear_nacl_from_fcport_map)(struct qla_tgt_sess *); - void (*put_sess)(struct qla_tgt_sess *); void (*shutdown_sess)(struct qla_tgt_sess *); }; @@ -930,6 +929,7 @@ struct qla_tgt_sess { int generation; struct se_session *se_sess; + struct kref sess_kref; struct scsi_qla_host *vha; struct qla_tgt *tgt; @@ -1092,7 +1092,7 @@ extern int qlt_remove_target(struct qla_hw_data *, struct scsi_qla_host *); extern int qlt_lport_register(void *, u64, u64, u64, int (*callback)(struct scsi_qla_host *, void *, u64, u64)); extern void qlt_lport_deregister(struct scsi_qla_host *); -extern void qlt_unreg_sess(struct qla_tgt_sess *); +void qlt_put_sess(struct qla_tgt_sess *sess); extern void qlt_fc_port_added(struct scsi_qla_host *, fc_port_t *); extern void qlt_fc_port_deleted(struct scsi_qla_host *, fc_port_t *, int); extern int __init qlt_init(void); diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index bf73c6f..6c8f643 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -361,7 +361,7 @@ static void tcm_qla2xxx_close_session(struct se_session *se_sess) vha = sess->vha; spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags); - qlt_unreg_sess(sess); + qlt_put_sess(sess); spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags); } @@ -688,23 +688,6 @@ static void tcm_qla2xxx_clear_nacl_from_fcport_map(struct qla_tgt_sess *sess) tcm_qla2xxx_clear_sess_lookup(lport, nacl, sess); } -static void tcm_qla2xxx_release_session(struct kref *kref) -{ - struct se_session *se_sess = container_of(kref, - struct se_session, sess_kref); - - qlt_unreg_sess(se_sess->fabric_sess_ptr); -} - -static void tcm_qla2xxx_put_sess(struct qla_tgt_sess *sess) -{ - if (!sess) - return; - - assert_spin_locked(&sess->vha->hw->tgt.sess_lock); - kref_put(&sess->se_sess->sess_kref, tcm_qla2xxx_release_session); -} - static void tcm_qla2xxx_shutdown_sess(struct qla_tgt_sess *sess) { assert_spin_locked(&sess->vha->hw->tgt.sess_lock); @@ -1509,7 +1492,6 @@ static struct qla_tgt_func_tmpl tcm_qla2xxx_template = { .find_sess_by_s_id = tcm_qla2xxx_find_sess_by_s_id, .find_sess_by_loop_id = tcm_qla2xxx_find_sess_by_loop_id, .clear_nacl_from_fcport_map = tcm_qla2xxx_clear_nacl_from_fcport_map, - .put_sess = tcm_qla2xxx_put_sess, .shutdown_sess = tcm_qla2xxx_shutdown_sess, }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html