re: qla2xxx: Add LLD target-mode infrastructure for >= 24xx series

["Nicholas A. Bellinger" <nab@xxxxxxxxxxxxxxx>]

On Mon, 2012-05-07 at 16:59 +0300, Dan Carpenter wrote:
> Hello Nicholas Bellinger,
> 
> This is a semi-automatic email about new static checker warnings.
> 

Hey Dan,

> The patch 2c0532cbbbb5: "qla2xxx: Add LLD target-mode infrastructure 
> for >= 24xx series" from May 3, 2012, leads to the following Smatch 
> complaint:
> 
> drivers/scsi/qla2xxx/qla_target.c:2965 qlt_abort_task()
> 	 error: we previously assumed 'sess' could be null (see line 2958)
> 
> drivers/scsi/qla2xxx/qla_target.c
>   2957		sess = ha->tgt.tgt_ops->find_sess_by_loop_id(vha, loop_id);
>   2958		if (sess == NULL) {
>                     ^^^^^^^^^^^^
> New check.
> 
>   2959			ql_dbg(ql_dbg_tgt_mgt, vha, 0xf025,
>   2960			    "qla_target(%d): task abort for unexisting "
>   2961			    "session\n", vha->vp_idx);
>   2962			res = qlt_sched_sess_work(ha->tgt.qla_tgt,
>   2963			    QLA_TGT_SESS_WORK_ABORT, iocb, sizeof(*iocb));
>   2964			if (res != 0)
>   2965				sess->tgt->tm_to_unknown = 1;
>                                 ^^^^^^^^^
> New dereference.
> 

So it looks like ->tm_to_unknown is actually left-over cruft before the
conversion to proper workqueue usage in qla_target.c, and now it's safe
to go ahead and drop this plus other unnecessary sess_works_pending
bitfield..

I'm committing the following patch into lio-core.git, and will fold into
for-next-merge for reference.

Qlogic folks, please also fold into your linux-scsi submission, or let
me know if you would prefer an updated target series.

Thanks!

--nab

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 2baef81..d1d5380 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -421,7 +421,6 @@ static int qlt_reset(struct scsi_qla_host *vha, void *iocb, int mcmd)
 	    "Using sess for qla_tgt_reset: %p\n", sess);
 	if (!sess) {
 		res = -ESRCH;
-		ha->tgt.qla_tgt->tm_to_unknown = 1;
 		return res;
 	}
 
@@ -1066,10 +1065,7 @@ static int qlt_sched_sess_work(struct qla_tgt *tgt, int type,
 	memcpy(&prm->tm_iocb, param, param_size);
 
 	spin_lock_irqsave(&tgt->sess_work_lock, flags);
-	if (!tgt->sess_works_pending)
-		tgt->tm_to_unknown = 0;
 	list_add_tail(&prm->sess_works_list_entry, &tgt->sess_works_list);
-	tgt->sess_works_pending = 1;
 	spin_unlock_irqrestore(&tgt->sess_work_lock, flags);
 
 	schedule_work(&tgt->sess_work);
@@ -1343,7 +1339,6 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 		rc = qlt_sched_sess_work(ha->tgt.qla_tgt,
 		    QLA_TGT_SESS_WORK_ABORT, abts, sizeof(*abts));
 		if (rc != 0) {
-			ha->tgt.qla_tgt->tm_to_unknown = 1;
 			qlt_24xx_send_abts_resp(vha, abts, FCP_TMF_REJECTED,
 			    false);
 		}
@@ -2897,8 +2892,6 @@ static int qlt_handle_task_mgmt(struct scsi_qla_host *vha, void *iocb)
 		    "non-existant session\n", vha->vp_idx, fn);
 		res = qlt_sched_sess_work(tgt, QLA_TGT_SESS_WORK_TM, iocb,
 		    sizeof(atio_from_isp_t));
-		if (res != 0)
-			tgt->tm_to_unknown = 1;
 
 		return res;
 	}
@@ -2961,8 +2954,6 @@ static int qlt_abort_task(struct scsi_qla_host *vha, imm_ntfy_from_isp_t *iocb)
 		    "session\n", vha->vp_idx);
 		res = qlt_sched_sess_work(ha->tgt.qla_tgt,
 		    QLA_TGT_SESS_WORK_ABORT, iocb, sizeof(*iocb));
-		if (res != 0)
-			sess->tgt->tm_to_unknown = 1;
 
 		return res;
 	}
@@ -4254,7 +4245,6 @@ static void qlt_sess_work_fn(struct work_struct *work)
 {
 	struct qla_tgt *tgt = container_of(work, struct qla_tgt, sess_work);
 	struct scsi_qla_host *vha = tgt->vha;
-	struct qla_hw_data *ha = vha->hw;
 	unsigned long flags;
 
 	ql_dbg(ql_dbg_tgt_mgt, vha, 0xf000, "Sess work (tgt %p)", tgt);
@@ -4290,15 +4280,6 @@ static void qlt_sess_work_fn(struct work_struct *work)
 		kfree(prm);
 	}
 	spin_unlock_irqrestore(&tgt->sess_work_lock, flags);
-
-	spin_lock_irqsave(&ha->hardware_lock, flags);
-	spin_lock(&tgt->sess_work_lock);
-	if (list_empty(&tgt->sess_works_list)) {
-		tgt->sess_works_pending = 0;
-		tgt->tm_to_unknown = 0;
-	}
-	spin_unlock(&tgt->sess_work_lock);
-	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 }
 
 /* Must be called under tgt_host_action_mutex */
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index f0280ae..7d5849f 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -763,8 +763,6 @@ struct qla_tgt {
 	/* Target's flags, serialized by pha->hardware_lock */
 	unsigned int tgt_enable_64bit_addr:1; /* 64-bits PCI addr enabled */
 	unsigned int link_reinit_iocb_pending:1;
-	unsigned int tm_to_unknown:1; /* TM to unknown session was sent */
-	unsigned int sess_works_pending:1; /* there are sess_work entries */
 
 	/*
 	 * Protected by tgt_mutex AND hardware_lock for writing and tgt_mutex

--
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