[PATCH 0/13] Misc patches leading to target_submit_tmr

[PATCH 0/13] Misc patches leading to target_submit_tmr

[Andy Grover]

Hi nab, Kiran, and all,

Here are some cleanup patches to target, scsi, and tcm_fc. The biggest
change is adding a target_submit_tmr to match the new target_submit_cmd
helper function. I've only changed tcm_fc to use it so far, if it looks ok
I will change over the other fabrics.

I'm especially looking for review of the error handling changes in the
last few patches. Does transport_generic_free_cmd end up doing the
same thing as ft_sess_put, for example?

Thanks -- Regards -- Andy

The following changes since commit 7acd456a2fdae22d0f43db60f689ba3c57b69838:

  target: Fail INQUIRY commands with EVPD==0 but PAGE CODE!=0 (2012-01-17 22:54:29 -0800)

are available in the git repository at:
  git://fedorapeople.org/home/fedora/grover/public_git/linux-2.6.git for-nab

Andy Grover (13):
      scsi: Use struct scsi_lun in fc/fcp.h
      target: fix comment typos
      target: Remove unused struct se_queue_req
      target/iscsi: Remove unneeded wrapper functions
      target/fc: Simplify ft_send_work for tmr path
      target/fc: Remove cmd->cdb data member
      target: Inline struct se_tmr_req into se_cmd
      target/fc: Move core->fc code conversion earlier in ft_send_tm()
      target/fc: Call lookup_tmr_lun() for all TM types
      target/fc: Use transport_generic_free_cmd for ft_sess_put in ft_send_tm
      target: Add target_submit_tmr helper function
      target/fc: Use target_submit_tmr()
      target: Change target_submit_cmd() to return void

 drivers/infiniband/ulp/srpt/ib_srpt.c      |   18 +---
 drivers/scsi/bnx2fc/bnx2fc_io.c            |    4 +-
 drivers/scsi/libfc/fc_fcp.c                |    5 +-
 drivers/scsi/qla2xxx/tcm_qla2xxx.c         |   22 ++---
 drivers/target/iscsi/iscsi_target.c        |   12 +-
 drivers/target/iscsi/iscsi_target_device.c |   19 ----
 drivers/target/iscsi/iscsi_target_device.h |    2 -
 drivers/target/iscsi/iscsi_target_tmr.c    |    6 +-
 drivers/target/iscsi/iscsi_target_util.c   |    8 +-
 drivers/target/loopback/tcm_loop.c         |   14 +--
 drivers/target/target_core_device.c        |    2 +-
 drivers/target/target_core_internal.h      |    2 -
 drivers/target/target_core_tmr.c           |   20 +---
 drivers/target/target_core_transport.c     |   77 +++++++++++-----
 drivers/target/tcm_fc/tcm_fc.h             |    2 -
 drivers/target/tcm_fc/tfc_cmd.c            |  142 ++++++++--------------------
 include/scsi/fc/fc_fcp.h                   |    6 +-
 include/target/target_core_base.h          |   47 ++++-----
 include/target/target_core_fabric.h        |    7 +-
 19 files changed, 164 insertions(+), 251 deletions(-)

[PATCH 01/13] scsi: Use struct scsi_lun in fc/fcp.h

[Andy Grover]

This allows us to use scsilun_to_int without an ugly cast.

Fix up places that use scsilun_to_int on fcp->fc_lun accordingly.

In fc target, this leaves ft_cmd.lun unused, so remove it.

Signed-off-by: Andy Grover <agrover@redhat.com>

Conflicts:

	drivers/target/tcm_fc/tfc_cmd.c
---
 drivers/scsi/bnx2fc/bnx2fc_io.c |    4 +---
 drivers/scsi/libfc/fc_fcp.c     |    5 ++---
 drivers/target/tcm_fc/tcm_fc.h  |    1 -
 drivers/target/tcm_fc/tfc_cmd.c |   11 ++++-------
 include/scsi/fc/fc_fcp.h        |    6 ++++--
 5 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 84a78af..e897ce9 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1682,9 +1682,7 @@ void bnx2fc_build_fcp_cmnd(struct bnx2fc_cmd *io_req,
 
 	memset(fcp_cmnd, 0, sizeof(struct fcp_cmnd));
 
-	int_to_scsilun(sc_cmd->device->lun,
-			(struct scsi_lun *) fcp_cmnd->fc_lun);
-
+	int_to_scsilun(sc_cmd->device->lun, &fcp_cmnd->fc_lun);
 
 	fcp_cmnd->fc_dl = htonl(io_req->data_xfer_len);
 	memcpy(fcp_cmnd->fc_cdb, sc_cmd->cmnd, sc_cmd->cmd_len);
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 221875e..94ada83 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1073,8 +1073,7 @@ static int fc_fcp_pkt_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp)
 	fsp->cdb_cmd.fc_dl = htonl(fsp->data_len);
 	fsp->cdb_cmd.fc_flags = fsp->req_flags & ~FCP_CFL_LEN_MASK;
 
-	int_to_scsilun(fsp->cmd->device->lun,
-		       (struct scsi_lun *)fsp->cdb_cmd.fc_lun);
+	int_to_scsilun(fsp->cmd->device->lun, &fsp->cdb_cmd.fc_lun);
 	memcpy(fsp->cdb_cmd.fc_cdb, fsp->cmd->cmnd, fsp->cmd->cmd_len);
 
 	spin_lock_irqsave(&si->scsi_queue_lock, flags);
@@ -1256,7 +1255,7 @@ static int fc_lun_reset(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
 
 	fsp->cdb_cmd.fc_dl = htonl(fsp->data_len);
 	fsp->cdb_cmd.fc_tm_flags = FCP_TMF_LUN_RESET;
-	int_to_scsilun(lun, (struct scsi_lun *)fsp->cdb_cmd.fc_lun);
+	int_to_scsilun(lun, &fsp->cdb_cmd.fc_lun);
 
 	fsp->wait_for_comp = 1;
 	init_completion(&fsp->tm_done);
diff --git a/drivers/target/tcm_fc/tcm_fc.h b/drivers/target/tcm_fc/tcm_fc.h
index 26061f0..3c1a756 100644
--- a/drivers/target/tcm_fc/tcm_fc.h
+++ b/drivers/target/tcm_fc/tcm_fc.h
@@ -113,7 +113,6 @@ struct ft_lport_acl {
  * Commands
  */
 struct ft_cmd {
-	u32 lun;                        /* LUN from request */
 	struct ft_sess *sess;		/* session held for cmd */
 	struct fc_seq *seq;		/* sequence in exchange mgr */
 	struct se_cmd se_cmd;		/* Local TCM I/O descriptor */
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 31f66c6..941fe00 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -61,7 +61,6 @@ void ft_dump_cmd(struct ft_cmd *cmd, const char *caller)
 		caller, cmd, cmd->sess, cmd->seq, se_cmd);
 	pr_debug("%s: cmd %p cdb %p\n",
 		caller, cmd, cmd->cdb);
-	pr_debug("%s: cmd %p lun %d\n", caller, cmd, cmd->lun);
 
 	pr_debug("%s: cmd %p data_nents %u len %u se_cmd_flags <0x%x>\n",
 		caller, cmd, se_cmd->t_data_nents,
@@ -407,8 +406,7 @@ static void ft_send_tm(struct ft_cmd *cmd)
 
 	switch (fcp->fc_tm_flags) {
 	case FCP_TMF_LUN_RESET:
-		cmd->lun = scsilun_to_int((struct scsi_lun *)fcp->fc_lun);
-		if (transport_lookup_tmr_lun(&cmd->se_cmd, cmd->lun) < 0) {
+		if (transport_lookup_tmr_lun(&cmd->se_cmd, scsilun_to_int(&fcp->fc_lun)) < 0) {
 			/*
 			 * Make sure to clean up newly allocated TMR request
 			 * since "unable to  handle TMR request because failed
@@ -416,7 +414,7 @@ static void ft_send_tm(struct ft_cmd *cmd)
 			 */
 			pr_debug("Failed to get LUN for TMR func %d, "
 				  "se_cmd %p, unpacked_lun %d\n",
-				  tm_func, &cmd->se_cmd, cmd->lun);
+				  tm_func, &cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
 			ft_dump_cmd(cmd, __func__);
 			sess = cmd->sess;
 			transport_send_check_condition_and_sense(&cmd->se_cmd,
@@ -596,14 +594,13 @@ static void ft_send_work(struct work_struct *work)
 		return;
 	}
 	fc_seq_exch(cmd->seq)->lp->tt.seq_set_resp(cmd->seq, ft_recv_seq, cmd);
-	cmd->lun = scsilun_to_int((struct scsi_lun *)fcp->fc_lun);
 	/*
  	 * Use a single se_cmd->cmd_kref as we expect to release se_cmd
  	 * directly from ft_check_stop_free callback in response path.
  	 */
 	ret = target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, cmd->cdb,
-				&cmd->ft_sense_buffer[0], cmd->lun, data_len,
-				task_attr, data_dir, 0);
+				&cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
+				data_len, task_attr, data_dir, 0);
 	pr_debug("r_ctl %x alloc target_submit_cmd %d\n", fh->fh_r_ctl, ret);
 	if (ret < 0) {
 		ft_dump_cmd(cmd, __func__);
diff --git a/include/scsi/fc/fc_fcp.h b/include/scsi/fc/fc_fcp.h
index 652dec2..0d7d67e 100644
--- a/include/scsi/fc/fc_fcp.h
+++ b/include/scsi/fc/fc_fcp.h
@@ -20,6 +20,8 @@
 #ifndef _FC_FCP_H_
 #define	_FC_FCP_H_
 
+#include <scsi/scsi.h>
+
 /*
  * Fibre Channel Protocol for SCSI.
  * From T10 FCP-3, T10 project 1560-D Rev 4, Sept. 13, 2005.
@@ -45,7 +47,7 @@
  * FCP_CMND IU Payload.
  */
 struct fcp_cmnd {
-	__u8		fc_lun[8];	/* logical unit number */
+	struct scsi_lun	fc_lun;		/* logical unit number */
 	__u8		fc_cmdref;	/* command reference number */
 	__u8		fc_pri_ta;	/* priority and task attribute */
 	__u8		fc_tm_flags;	/* task management flags */
@@ -57,7 +59,7 @@ struct fcp_cmnd {
 #define	FCP_CMND_LEN	32	/* expected length of structure */
 
 struct fcp_cmnd32 {
-	__u8		fc_lun[8];	/* logical unit number */
+	struct scsi_lun	fc_lun;		/* logical unit number */
 	__u8		fc_cmdref;	/* command reference number */
 	__u8		fc_pri_ta;	/* priority and task attribute */
 	__u8		fc_tm_flags;	/* task management flags */
-- 
1.7.1

[PATCH 02/13] target: fix comment typos

[Andy Grover]

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 include/target/target_core_base.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 120c414..6149b2a 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -592,12 +592,12 @@ struct se_cmd {
 };
 
 struct se_tmr_req {
-	/* Task Management function to be preformed */
+	/* Task Management function to be performed */
 	u8			function;
 	/* Task Management response to send */
 	u8			response;
 	int			call_transport;
-	/* Reference to ITT that Task Mgmt should be preformed */
+	/* Reference to ITT that Task Mgmt should be performed */
 	u32			ref_task_tag;
 	/* 64-bit encoded SAM LUN from $FABRIC_MOD TMR header */
 	u64			ref_task_lun;
-- 
1.7.1

[PATCH 03/13] target: Remove unused struct se_queue_req

[Andy Grover]

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 include/target/target_core_base.h |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 6149b2a..bb739f9 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -473,12 +473,6 @@ struct t10_reservation {
 	struct t10_reservation_ops pr_ops;
 };
 
-struct se_queue_req {
-	int			state;
-	struct se_cmd		*cmd;
-	struct list_head	qr_list;
-};
-
 struct se_queue_obj {
 	atomic_t		queue_cnt;
 	spinlock_t		cmd_queue_lock;
-- 
1.7.1

[PATCH 04/13] target/iscsi: Remove unneeded wrapper functions

[Andy Grover]

iscsit_get_lun_for_{cmd,tmr} are unnecessary.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/iscsi/iscsi_target.c        |    8 ++++----
 drivers/target/iscsi/iscsi_target_device.c |   19 -------------------
 drivers/target/iscsi/iscsi_target_device.h |    2 --
 3 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 357a6f2..c1a5e09 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1006,8 +1006,8 @@ done:
 	/*
 	 * The CDB is going to an se_device_t.
 	 */
-	ret = iscsit_get_lun_for_cmd(cmd, hdr->cdb,
-				get_unaligned_le64(&hdr->lun));
+	ret = transport_lookup_cmd_lun(&cmd->se_cmd,
+				       scsilun_to_int(&hdr->lun));
 	if (ret < 0) {
 		if (cmd->se_cmd.scsi_sense_reason == TCM_NON_EXISTENT_LUN) {
 			pr_debug("Responding to non-acl'ed,"
@@ -1746,8 +1746,8 @@ static int iscsit_handle_task_mgt_cmd(
 	 * Locate the struct se_lun for all TMRs not related to ERL=2 TASK_REASSIGN
 	 */
 	if (function != ISCSI_TM_FUNC_TASK_REASSIGN) {
-		ret = iscsit_get_lun_for_tmr(cmd,
-				get_unaligned_le64(&hdr->lun));
+		ret = transport_lookup_tmr_lun(&cmd->se_cmd,
+					       scsilun_to_int(&hdr->lun));
 		if (ret < 0) {
 			cmd->se_cmd.se_cmd_flags |= SCF_SCSI_CDB_EXCEPTION;
 			se_tmr->response = ISCSI_TMF_RSP_NO_LUN;
diff --git a/drivers/target/iscsi/iscsi_target_device.c b/drivers/target/iscsi/iscsi_target_device.c
index f63ea35..bcc4098 100644
--- a/drivers/target/iscsi/iscsi_target_device.c
+++ b/drivers/target/iscsi/iscsi_target_device.c
@@ -28,25 +28,6 @@
 #include "iscsi_target_tpg.h"
 #include "iscsi_target_util.h"
 
-int iscsit_get_lun_for_tmr(
-	struct iscsi_cmd *cmd,
-	u64 lun)
-{
-	u32 unpacked_lun = scsilun_to_int((struct scsi_lun *)&lun);
-
-	return transport_lookup_tmr_lun(&cmd->se_cmd, unpacked_lun);
-}
-
-int iscsit_get_lun_for_cmd(
-	struct iscsi_cmd *cmd,
-	unsigned char *cdb,
-	u64 lun)
-{
-	u32 unpacked_lun = scsilun_to_int((struct scsi_lun *)&lun);
-
-	return transport_lookup_cmd_lun(&cmd->se_cmd, unpacked_lun);
-}
-
 void iscsit_determine_maxcmdsn(struct iscsi_session *sess)
 {
 	struct se_node_acl *se_nacl;
diff --git a/drivers/target/iscsi/iscsi_target_device.h b/drivers/target/iscsi/iscsi_target_device.h
index bef1cad..a0e2df9 100644
--- a/drivers/target/iscsi/iscsi_target_device.h
+++ b/drivers/target/iscsi/iscsi_target_device.h
@@ -1,8 +1,6 @@
 #ifndef ISCSI_TARGET_DEVICE_H
 #define ISCSI_TARGET_DEVICE_H
 
-extern int iscsit_get_lun_for_tmr(struct iscsi_cmd *, u64);
-extern int iscsit_get_lun_for_cmd(struct iscsi_cmd *, unsigned char *, u64);
 extern void iscsit_determine_maxcmdsn(struct iscsi_session *);
 extern void iscsit_increment_maxcmdsn(struct iscsi_cmd *, struct iscsi_session *);
 
-- 
1.7.1

[PATCH 05/13] target/fc: Simplify ft_send_work for tmr path

[Andy Grover]

Check fc_tm_flags early and call ft_send_tm() right away. Don't need to
set local vars for tm case.

data_len local var now unneeded, remove.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_cmd.c |   76 ++++++++++++++++++---------------------
 1 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 941fe00..85fcd09 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -536,7 +536,6 @@ static void ft_send_work(struct work_struct *work)
 	struct fc_frame_header *fh = fc_frame_header_get(cmd->req_frame);
 	struct fcp_cmnd *fcp;
 	int data_dir = 0;
-	u32 data_len;
 	int task_attr;
 	int ret;
 
@@ -547,45 +546,6 @@ static void ft_send_work(struct work_struct *work)
 	if (fcp->fc_flags & FCP_CFL_LEN_MASK)
 		goto err;		/* not handling longer CDBs yet */
 
-	if (fcp->fc_tm_flags) {
-		task_attr = FCP_PTA_SIMPLE;
-		data_dir = DMA_NONE;
-		data_len = 0;
-	} else {
-		switch (fcp->fc_flags & (FCP_CFL_RDDATA | FCP_CFL_WRDATA)) {
-		case 0:
-			data_dir = DMA_NONE;
-			break;
-		case FCP_CFL_RDDATA:
-			data_dir = DMA_FROM_DEVICE;
-			break;
-		case FCP_CFL_WRDATA:
-			data_dir = DMA_TO_DEVICE;
-			break;
-		case FCP_CFL_WRDATA | FCP_CFL_RDDATA:
-			goto err;	/* TBD not supported by tcm_fc yet */
-		}
-		/*
-		 * Locate the SAM Task Attr from fc_pri_ta
-		 */
-		switch (fcp->fc_pri_ta & FCP_PTA_MASK) {
-		case FCP_PTA_HEADQ:
-			task_attr = MSG_HEAD_TAG;
-			break;
-		case FCP_PTA_ORDERED:
-			task_attr = MSG_ORDERED_TAG;
-			break;
-		case FCP_PTA_ACA:
-			task_attr = MSG_ACA_TAG;
-			break;
-		case FCP_PTA_SIMPLE: /* Fallthrough */
-		default:
-			task_attr = MSG_SIMPLE_TAG;
-		}
-		
-		data_len = ntohl(fcp->fc_dl);
-		cmd->cdb = fcp->fc_cdb;
-	}
 	/*
 	 * Check for FCP task management flags
 	 */
@@ -593,6 +553,40 @@ static void ft_send_work(struct work_struct *work)
 		ft_send_tm(cmd);
 		return;
 	}
+
+	switch (fcp->fc_flags & (FCP_CFL_RDDATA | FCP_CFL_WRDATA)) {
+	case 0:
+		data_dir = DMA_NONE;
+		break;
+	case FCP_CFL_RDDATA:
+		data_dir = DMA_FROM_DEVICE;
+		break;
+	case FCP_CFL_WRDATA:
+		data_dir = DMA_TO_DEVICE;
+		break;
+	case FCP_CFL_WRDATA | FCP_CFL_RDDATA:
+		goto err;	/* TBD not supported by tcm_fc yet */
+	}
+	/*
+	 * Locate the SAM Task Attr from fc_pri_ta
+	 */
+	switch (fcp->fc_pri_ta & FCP_PTA_MASK) {
+	case FCP_PTA_HEADQ:
+		task_attr = MSG_HEAD_TAG;
+		break;
+	case FCP_PTA_ORDERED:
+		task_attr = MSG_ORDERED_TAG;
+		break;
+	case FCP_PTA_ACA:
+		task_attr = MSG_ACA_TAG;
+		break;
+	case FCP_PTA_SIMPLE: /* Fallthrough */
+	default:
+		task_attr = MSG_SIMPLE_TAG;
+	}
+
+	cmd->cdb = fcp->fc_cdb;
+
 	fc_seq_exch(cmd->seq)->lp->tt.seq_set_resp(cmd->seq, ft_recv_seq, cmd);
 	/*
  	 * Use a single se_cmd->cmd_kref as we expect to release se_cmd
@@ -600,7 +594,7 @@ static void ft_send_work(struct work_struct *work)
  	 */
 	ret = target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, cmd->cdb,
 				&cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
-				data_len, task_attr, data_dir, 0);
+				ntohl(fcp->fc_dl), task_attr, data_dir, 0);
 	pr_debug("r_ctl %x alloc target_submit_cmd %d\n", fh->fh_r_ctl, ret);
 	if (ret < 0) {
 		ft_dump_cmd(cmd, __func__);
-- 
1.7.1

[PATCH 06/13] target/fc: Remove cmd->cdb data member

[Andy Grover]

It's used only for debug output. Debug output may want to make use of
fcp->fc_cdb directly.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tcm_fc.h  |    1 -
 drivers/target/tcm_fc/tfc_cmd.c |    8 +-------
 2 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/drivers/target/tcm_fc/tcm_fc.h b/drivers/target/tcm_fc/tcm_fc.h
index 3c1a756..650a2ba 100644
--- a/drivers/target/tcm_fc/tcm_fc.h
+++ b/drivers/target/tcm_fc/tcm_fc.h
@@ -117,7 +117,6 @@ struct ft_cmd {
 	struct fc_seq *seq;		/* sequence in exchange mgr */
 	struct se_cmd se_cmd;		/* Local TCM I/O descriptor */
 	struct fc_frame *req_frame;
-	unsigned char *cdb;		/* pointer to CDB inside frame */
 	u32 write_data_len;		/* data received on writes */
 	struct work_struct work;
 	/* Local sense buffer */
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 85fcd09..6884b0f 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -59,8 +59,6 @@ void ft_dump_cmd(struct ft_cmd *cmd, const char *caller)
 	se_cmd = &cmd->se_cmd;
 	pr_debug("%s: cmd %p sess %p seq %p se_cmd %p\n",
 		caller, cmd, cmd->sess, cmd->seq, se_cmd);
-	pr_debug("%s: cmd %p cdb %p\n",
-		caller, cmd, cmd->cdb);
 
 	pr_debug("%s: cmd %p data_nents %u len %u se_cmd_flags <0x%x>\n",
 		caller, cmd, se_cmd->t_data_nents,
@@ -80,8 +78,6 @@ void ft_dump_cmd(struct ft_cmd *cmd, const char *caller)
 			caller, cmd, ep->sid, ep->did, ep->oxid, ep->rxid,
 			sp->id, ep->esb_stat);
 	}
-	print_hex_dump(KERN_INFO, "ft_dump_cmd ", DUMP_PREFIX_NONE,
-		16, 4, cmd->cdb, MAX_COMMAND_SIZE, 0);
 }
 
 static void ft_free_cmd(struct ft_cmd *cmd)
@@ -585,14 +581,12 @@ static void ft_send_work(struct work_struct *work)
 		task_attr = MSG_SIMPLE_TAG;
 	}
 
-	cmd->cdb = fcp->fc_cdb;
-
 	fc_seq_exch(cmd->seq)->lp->tt.seq_set_resp(cmd->seq, ft_recv_seq, cmd);
 	/*
  	 * Use a single se_cmd->cmd_kref as we expect to release se_cmd
  	 * directly from ft_check_stop_free callback in response path.
  	 */
-	ret = target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, cmd->cdb,
+	ret = target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, fcp->fc_cdb,
 				&cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
 				ntohl(fcp->fc_dl), task_attr, data_dir, 0);
 	pr_debug("r_ctl %x alloc target_submit_cmd %d\n", fh->fh_r_ctl, ret);
-- 
1.7.1

[PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Andy Grover]

This saves all fabrics from calling core_tmr_alloc_req() and
having to check the result. The downside is se_cmd gets bigger for all
requests, but hopefully later patches will reduce it.

Change the test for if a cmd is a tmr request to checking if
SCF_SCSI_TMR_CDB (a new flag) is set in cmd->se_cmd_flags.

Fixup fabric accesses of se_tmr_req

Change core_tmr_alloc_req into core_tmr_req_init

Remove se_tmr_req_cache

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c    |   13 +++------
 drivers/scsi/qla2xxx/tcm_qla2xxx.c       |   15 +++++------
 drivers/target/iscsi/iscsi_target.c      |    4 +-
 drivers/target/iscsi/iscsi_target_tmr.c  |    6 ++--
 drivers/target/iscsi/iscsi_target_util.c |    8 +----
 drivers/target/loopback/tcm_loop.c       |   14 ++++------
 drivers/target/target_core_device.c      |    2 +-
 drivers/target/target_core_internal.h    |    2 -
 drivers/target/target_core_tmr.c         |   20 +++-----------
 drivers/target/target_core_transport.c   |   30 +++++++--------------
 drivers/target/tcm_fc/tfc_cmd.c          |   11 +------
 include/target/target_core_base.h        |   41 +++++++++++++++--------------
 include/target/target_core_fabric.h      |    2 +-
 13 files changed, 64 insertions(+), 104 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index ecd2076..5ec4f9d 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1797,19 +1797,14 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch,
 	tcm_tmr = srp_tmr_to_tcm(srp_tsk->tsk_mgmt_func);
 	if (tcm_tmr < 0) {
 		send_ioctx->cmd.se_cmd_flags |= SCF_SCSI_CDB_EXCEPTION;
-		send_ioctx->cmd.se_tmr_req->response =
+		send_ioctx->cmd.se_tmr_req.response =
 			TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED;
 		goto process_tmr;
 	}
 	transport_init_se_cmd(&send_ioctx->cmd, &srpt_target->tf_ops, ch->sess,
 			0, DMA_NONE, MSG_SIMPLE_TAG, send_ioctx->sense_data);
 
-	cmd->se_tmr_req = core_tmr_alloc_req(cmd, NULL, tcm_tmr, GFP_KERNEL);
-	if (!cmd->se_tmr_req) {
-		send_ioctx->cmd.se_cmd_flags |= SCF_SCSI_CDB_EXCEPTION;
-		send_ioctx->cmd.se_tmr_req->response = TMR_FUNCTION_REJECTED;
-		goto process_tmr;
-	}
+	core_tmr_req_init(cmd, NULL, tcm_tmr);
 
 	unpacked_lun = srpt_unpack_lun((uint8_t *)&srp_tsk->lun,
 				       sizeof(srp_tsk->lun));
@@ -1817,7 +1812,7 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch,
 	if (res) {
 		pr_debug("rejecting TMR for LUN %lld\n", unpacked_lun);
 		send_ioctx->cmd.se_cmd_flags |= SCF_SCSI_CDB_EXCEPTION;
-		send_ioctx->cmd.se_tmr_req->response = TMR_LUN_DOES_NOT_EXIST;
+		send_ioctx->cmd.se_tmr_req.response = TMR_LUN_DOES_NOT_EXIST;
 		goto process_tmr;
 	}
 
@@ -3011,7 +3006,7 @@ static int srpt_queue_response(struct se_cmd *cmd)
 					      cmd->scsi_status);
 	else {
 		srp_tm_status
-			= tcm_to_srp_tsk_mgmt_status(cmd->se_tmr_req->response);
+			= tcm_to_srp_tsk_mgmt_status(cmd->se_tmr_req.response);
 		resp_len = srpt_build_tskmgmt_rsp(ch, ioctx, srp_tm_status,
 						 ioctx->tag);
 	}
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index f92f64b..7132c1c 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -410,7 +410,7 @@ void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd *cmd)
  */
 int tcm_qla2xxx_check_stop_free(struct se_cmd *se_cmd)
 {
-	if (se_cmd->se_tmr_req) {
+	if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) {
 		struct qla_tgt_mgmt_cmd *mcmd = container_of(se_cmd,
 				struct qla_tgt_mgmt_cmd, se_cmd);
 		/*
@@ -431,7 +431,7 @@ void tcm_qla2xxx_release_cmd(struct se_cmd *se_cmd)
 {
 	struct qla_tgt_cmd *cmd;
 
-	if (se_cmd->se_tmr_req != NULL)
+	if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
 		return;
 
 	cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
@@ -684,15 +684,14 @@ int tcm_qla2xxx_handle_tmr(struct qla_tgt_mgmt_cmd *mcmd, uint32_t lun, uint8_t
 	transport_init_se_cmd(se_cmd, se_tpg->se_tpg_tfo, se_sess, 0,
 				DMA_NONE, 0, NULL);
 	/*
-	 * Allocate the TCM TMR
+	 * Initialize the TCM TMR
 	 */
-	se_cmd->se_tmr_req = core_tmr_alloc_req(se_cmd, mcmd, tmr_func, GFP_ATOMIC);
-	if (!se_cmd->se_tmr_req)
-		return -ENOMEM;
+	core_tmr_req_init(se_cmd, mcmd, tmr_func);
+
 	/*
 	 * Save the se_tmr_req for qla_tgt_xmit_tm_rsp() callback into LLD code
 	 */
-	mcmd->se_tmr_req = se_cmd->se_tmr_req;
+	mcmd->se_tmr_req = &se_cmd->se_tmr_req;
 	/*
 	 * Locate the underlying TCM struct se_lun from sc->device->lun
 	 */
@@ -759,7 +758,7 @@ int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
 
 int tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
 {
-	struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
+	struct se_tmr_req *se_tmr = &se_cmd->se_tmr_req;
 	struct qla_tgt_mgmt_cmd *mcmd = container_of(se_cmd,
 				struct qla_tgt_mgmt_cmd, se_cmd);
 
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index c1a5e09..9cd2837 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1740,7 +1740,7 @@ static int iscsit_handle_task_mgt_cmd(
 	cmd->targ_xfer_tag	= 0xFFFFFFFF;
 	cmd->cmd_sn		= hdr->cmdsn;
 	cmd->exp_stat_sn	= hdr->exp_statsn;
-	se_tmr			= cmd->se_cmd.se_tmr_req;
+	se_tmr			= &cmd->se_cmd.se_tmr_req;
 	tmr_req			= cmd->tmr_req;
 	/*
 	 * Locate the struct se_lun for all TMRs not related to ERL=2 TASK_REASSIGN
@@ -3120,7 +3120,7 @@ static int iscsit_send_task_mgt_rsp(
 	struct iscsi_cmd *cmd,
 	struct iscsi_conn *conn)
 {
-	struct se_tmr_req *se_tmr = cmd->se_cmd.se_tmr_req;
+	struct se_tmr_req *se_tmr = &cmd->se_cmd.se_tmr_req;
 	struct iscsi_tm_rsp *hdr;
 	u32 tx_size = 0;
 
diff --git a/drivers/target/iscsi/iscsi_target_tmr.c b/drivers/target/iscsi/iscsi_target_tmr.c
index e01da9d..438fa67 100644
--- a/drivers/target/iscsi/iscsi_target_tmr.c
+++ b/drivers/target/iscsi/iscsi_target_tmr.c
@@ -42,7 +42,7 @@ u8 iscsit_tmr_abort_task(
 	struct iscsi_cmd *ref_cmd;
 	struct iscsi_conn *conn = cmd->conn;
 	struct iscsi_tmr_req *tmr_req = cmd->tmr_req;
-	struct se_tmr_req *se_tmr = cmd->se_cmd.se_tmr_req;
+	struct se_tmr_req *se_tmr = &cmd->se_cmd.se_tmr_req;
 	struct iscsi_tm *hdr = (struct iscsi_tm *) buf;
 
 	ref_cmd = iscsit_find_cmd_from_itt(conn, hdr->rtt);
@@ -122,7 +122,7 @@ u8 iscsit_tmr_task_reassign(
 	struct iscsi_conn *conn = cmd->conn;
 	struct iscsi_conn_recovery *cr = NULL;
 	struct iscsi_tmr_req *tmr_req = cmd->tmr_req;
-	struct se_tmr_req *se_tmr = cmd->se_cmd.se_tmr_req;
+	struct se_tmr_req *se_tmr = &cmd->se_cmd.se_tmr_req;
 	struct iscsi_tm *hdr = (struct iscsi_tm *) buf;
 	int ret;
 
@@ -459,7 +459,7 @@ static int iscsit_task_reassign_complete(
 extern int iscsit_tmr_post_handler(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
 {
 	struct iscsi_tmr_req *tmr_req = cmd->tmr_req;
-	struct se_tmr_req *se_tmr = cmd->se_cmd.se_tmr_req;
+	struct se_tmr_req *se_tmr = &cmd->se_cmd.se_tmr_req;
 
 	if (tmr_req->task_reassign &&
 	   (se_tmr->response == ISCSI_TMF_RSP_COMPLETE))
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 11287e1..2a15d02 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -286,13 +286,9 @@ struct iscsi_cmd *iscsit_allocate_se_cmd_for_tmr(
 		goto out;
 	}
 
-	se_cmd->se_tmr_req = core_tmr_alloc_req(se_cmd,
-				cmd->tmr_req, tcm_function,
-				GFP_KERNEL);
-	if (!se_cmd->se_tmr_req)
-		goto out;
+	core_tmr_req_init(se_cmd, cmd->tmr_req, tcm_function);
 
-	cmd->tmr_req->se_tmr_req = se_cmd->se_tmr_req;
+	cmd->tmr_req->se_tmr_req = &se_cmd->se_tmr_req;
 
 	return cmd;
 out:
diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 5f69594..8302f01 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -187,7 +187,7 @@ static int tcm_loop_check_stop_free(struct se_cmd *se_cmd)
 	 * pointer.  These will be released directly in tcm_loop_device_reset()
 	 * with transport_generic_free_cmd().
 	 */
-	if (se_cmd->se_tmr_req)
+	if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
 		return 0;
 	/*
 	 * Release the struct se_cmd, which will make a callback to release
@@ -366,12 +366,10 @@ static int tcm_loop_device_reset(struct scsi_cmnd *sc)
 				DMA_NONE, MSG_SIMPLE_TAG,
 				&tl_cmd->tl_sense_buf[0]);
 	/*
-	 * Allocate the LUN_RESET TMR
+	 * Initialize the LUN_RESET TMR
 	 */
-	se_cmd->se_tmr_req = core_tmr_alloc_req(se_cmd, tl_tmr,
-						TMR_LUN_RESET, GFP_KERNEL);
-	if (IS_ERR(se_cmd->se_tmr_req))
-		goto release;
+	core_tmr_req_init(se_cmd, tl_tmr, TMR_LUN_RESET);
+
 	/*
 	 * Locate the underlying TCM struct se_lun from sc->device->lun
 	 */
@@ -387,7 +385,7 @@ static int tcm_loop_device_reset(struct scsi_cmnd *sc)
 	 * The TMR LUN_RESET has completed, check the response status and
 	 * then release allocations.
 	 */
-	ret = (se_cmd->se_tmr_req->response == TMR_FUNCTION_COMPLETE) ?
+	ret = (se_cmd->se_tmr_req.response == TMR_FUNCTION_COMPLETE) ?
 		SUCCESS : FAILED;
 release:
 	if (se_cmd)
@@ -886,7 +884,7 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
 
 static int tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
 {
-	struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
+	struct se_tmr_req *se_tmr = &se_cmd->se_tmr_req;
 	struct tcm_loop_tmr *tl_tmr = se_tmr->fabric_tmr_ptr;
 	/*
 	 * The SCSI EH thread will be sleeping on se_tmr->tl_tmr_wait, go ahead
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 6432685..d94f99c 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -173,7 +173,7 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
 	struct se_dev_entry *deve;
 	struct se_lun *se_lun = NULL;
 	struct se_session *se_sess = se_cmd->se_sess;
-	struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
+	struct se_tmr_req *se_tmr = &se_cmd->se_tmr_req;
 	unsigned long flags;
 
 	if (unpacked_lun >= TRANSPORT_MAX_LUNS_PER_TPG) {
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 4500136..e414c90 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -94,8 +94,6 @@ struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32 unpacked_lun);
 int	core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
 
 /* target_core_transport.c */
-extern struct kmem_cache *se_tmr_req_cache;
-
 int	init_se_kmem_caches(void);
 void	release_se_kmem_caches(void);
 u32	scsi_get_new_index(scsi_index_t);
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index a5c2e41..4c91ad0 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -40,27 +40,20 @@
 #include "target_core_alua.h"
 #include "target_core_pr.h"
 
-struct se_tmr_req *core_tmr_alloc_req(
+void core_tmr_req_init(
 	struct se_cmd *se_cmd,
 	void *fabric_tmr_ptr,
-	u8 function,
-	gfp_t gfp_flags)
+	u8 function)
 {
-	struct se_tmr_req *tmr;
+	struct se_tmr_req *tmr = &se_cmd->se_tmr_req;
 
-	tmr = kmem_cache_zalloc(se_tmr_req_cache, gfp_flags);
-	if (!tmr) {
-		pr_err("Unable to allocate struct se_tmr_req\n");
-		return ERR_PTR(-ENOMEM);
-	}
+	se_cmd->se_cmd_flags |= SCF_SCSI_TMR_CDB;
 	tmr->task_cmd = se_cmd;
 	tmr->fabric_tmr_ptr = fabric_tmr_ptr;
 	tmr->function = function;
 	INIT_LIST_HEAD(&tmr->tmr_list);
-
-	return tmr;
 }
-EXPORT_SYMBOL(core_tmr_alloc_req);
+EXPORT_SYMBOL(core_tmr_req_init);
 
 void core_tmr_release_req(
 	struct se_tmr_req *tmr)
@@ -69,15 +62,12 @@ void core_tmr_release_req(
 	unsigned long flags;
 
 	if (!dev) {
-		kmem_cache_free(se_tmr_req_cache, tmr);
 		return;
 	}
 
 	spin_lock_irqsave(&dev->se_tmr_lock, flags);
 	list_del(&tmr->tmr_list);
 	spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
-
-	kmem_cache_free(se_tmr_req_cache, tmr);
 }
 
 static void core_tmr_handle_tas_abort(
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 5d8867c..314567a 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -58,7 +58,6 @@ static int sub_api_initialized;
 
 static struct workqueue_struct *target_completion_wq;
 static struct kmem_cache *se_sess_cache;
-struct kmem_cache *se_tmr_req_cache;
 struct kmem_cache *se_ua_cache;
 struct kmem_cache *t10_pr_reg_cache;
 struct kmem_cache *t10_alua_lu_gp_cache;
@@ -82,21 +81,13 @@ static void target_complete_ok_work(struct work_struct *work);
 
 int init_se_kmem_caches(void)
 {
-	se_tmr_req_cache = kmem_cache_create("se_tmr_cache",
-			sizeof(struct se_tmr_req), __alignof__(struct se_tmr_req),
-			0, NULL);
-	if (!se_tmr_req_cache) {
-		pr_err("kmem_cache_create() for struct se_tmr_req"
-				" failed\n");
-		goto out;
-	}
 	se_sess_cache = kmem_cache_create("se_sess_cache",
 			sizeof(struct se_session), __alignof__(struct se_session),
 			0, NULL);
 	if (!se_sess_cache) {
 		pr_err("kmem_cache_create() for struct se_session"
 				" failed\n");
-		goto out_free_tmr_req_cache;
+		goto out;
 	}
 	se_ua_cache = kmem_cache_create("se_ua_cache",
 			sizeof(struct se_ua), __alignof__(struct se_ua),
@@ -169,8 +160,6 @@ out_free_ua_cache:
 	kmem_cache_destroy(se_ua_cache);
 out_free_sess_cache:
 	kmem_cache_destroy(se_sess_cache);
-out_free_tmr_req_cache:
-	kmem_cache_destroy(se_tmr_req_cache);
 out:
 	return -ENOMEM;
 }
@@ -178,7 +167,6 @@ out:
 void release_se_kmem_caches(void)
 {
 	destroy_workqueue(target_completion_wq);
-	kmem_cache_destroy(se_tmr_req_cache);
 	kmem_cache_destroy(se_sess_cache);
 	kmem_cache_destroy(se_ua_cache);
 	kmem_cache_destroy(t10_pr_reg_cache);
@@ -555,7 +543,7 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd)
 
 void transport_cmd_finish_abort(struct se_cmd *cmd, int remove)
 {
-	if (!cmd->se_tmr_req)
+	if (!(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
 		transport_lun_remove_cmd(cmd);
 
 	if (transport_cmd_check_stop_to_fabric(cmd))
@@ -3942,8 +3930,8 @@ void transport_release_cmd(struct se_cmd *cmd)
 {
 	BUG_ON(!cmd->se_tfo);
 
-	if (cmd->se_tmr_req)
-		core_tmr_release_req(cmd->se_tmr_req);
+	if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
+		core_tmr_release_req(&cmd->se_tmr_req);
 	if (cmd->t_task_cdb != cmd->__t_task_cdb)
 		kfree(cmd->t_task_cdb);
 	/*
@@ -3961,7 +3949,7 @@ EXPORT_SYMBOL(transport_release_cmd);
 void transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
 {
 	if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) {
-		if (wait_for_tasks && cmd->se_tmr_req)
+		if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
 			 transport_wait_for_tasks(cmd);
 
 		transport_release_cmd(cmd);
@@ -4287,7 +4275,8 @@ bool transport_wait_for_tasks(struct se_cmd *cmd)
 	unsigned long flags;
 
 	spin_lock_irqsave(&cmd->t_state_lock, flags);
-	if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD) && !(cmd->se_tmr_req)) {
+	if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD) &&
+	    !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) {
 		spin_unlock_irqrestore(&cmd->t_state_lock, flags);
 		return false;
 	}
@@ -4295,7 +4284,8 @@ bool transport_wait_for_tasks(struct se_cmd *cmd)
 	 * Only perform a possible wait_for_tasks if SCF_SUPPORTED_SAM_OPCODE
 	 * has been set in transport_set_supported_SAM_opcode().
 	 */
-	if (!(cmd->se_cmd_flags & SCF_SUPPORTED_SAM_OPCODE) && !cmd->se_tmr_req) {
+	if (!(cmd->se_cmd_flags & SCF_SUPPORTED_SAM_OPCODE) &&
+	    !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) {
 		spin_unlock_irqrestore(&cmd->t_state_lock, flags);
 		return false;
 	}
@@ -4639,7 +4629,7 @@ void transport_send_task_abort(struct se_cmd *cmd)
 static int transport_generic_do_tmr(struct se_cmd *cmd)
 {
 	struct se_device *dev = cmd->se_dev;
-	struct se_tmr_req *tmr = cmd->se_tmr_req;
+	struct se_tmr_req *tmr = &cmd->se_tmr_req;
 	int ret;
 
 	switch (tmr->function) {
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 6884b0f..f7dad73 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -353,7 +353,6 @@ static void ft_send_resp_code_and_free(struct ft_cmd *cmd,
  */
 static void ft_send_tm(struct ft_cmd *cmd)
 {
-	struct se_tmr_req *tmr;
 	struct fcp_cmnd *fcp;
 	struct ft_sess *sess;
 	u8 tm_func;
@@ -392,13 +391,7 @@ static void ft_send_tm(struct ft_cmd *cmd)
 	}
 
 	pr_debug("alloc tm cmd fn %d\n", tm_func);
-	tmr = core_tmr_alloc_req(&cmd->se_cmd, cmd, tm_func, GFP_KERNEL);
-	if (!tmr) {
-		pr_debug("alloc failed\n");
-		ft_send_resp_code_and_free(cmd, FCP_TMF_FAILED);
-		return;
-	}
-	cmd->se_cmd.se_tmr_req = tmr;
+	core_tmr_req_init(&cmd->se_cmd, cmd, tm_func);
 
 	switch (fcp->fc_tm_flags) {
 	case FCP_TMF_LUN_RESET:
@@ -436,7 +429,7 @@ static void ft_send_tm(struct ft_cmd *cmd)
 int ft_queue_tm_resp(struct se_cmd *se_cmd)
 {
 	struct ft_cmd *cmd = container_of(se_cmd, struct ft_cmd, se_cmd);
-	struct se_tmr_req *tmr = se_cmd->se_tmr_req;
+	struct se_tmr_req *tmr = &se_cmd->se_tmr_req;
 	enum fcp_resp_rsp_codes code;
 
 	switch (tmr->response) {
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index bb739f9..7ee42d1 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -168,7 +168,8 @@ enum se_cmd_flags_table {
 	SCF_EMULATED_TASK_SENSE		= 0x00000004,
 	SCF_SCSI_DATA_SG_IO_CDB		= 0x00000008,
 	SCF_SCSI_CONTROL_SG_IO_CDB	= 0x00000010,
-	SCF_SCSI_NON_DATA_CDB		= 0x00000040,
+	SCF_SCSI_NON_DATA_CDB		= 0x00000020,
+	SCF_SCSI_TMR_CDB		= 0x00000040,
 	SCF_SCSI_CDB_EXCEPTION		= 0x00000080,
 	SCF_SCSI_RESERVATION_CONFLICT	= 0x00000100,
 	SCF_FUA				= 0x00000200,
@@ -497,6 +498,24 @@ struct se_task {
 	struct completion	task_stop_comp;
 };
 
+struct se_tmr_req {
+	/* Task Management function to be performed */
+	u8			function;
+	/* Task Management response to send */
+	u8			response;
+	int			call_transport;
+	/* Reference to ITT that Task Mgmt should be performed */
+	u32			ref_task_tag;
+	/* 64-bit encoded SAM LUN from $FABRIC_MOD TMR header */
+	u64			ref_task_lun;
+	void 			*fabric_tmr_ptr;
+	struct se_cmd		*task_cmd;
+	struct se_cmd		*ref_cmd;
+	struct se_device	*tmr_dev;
+	struct se_lun		*tmr_lun;
+	struct list_head	tmr_list;
+};
+
 struct se_cmd {
 	/* SAM response code being sent to initiator */
 	u8			scsi_status;
@@ -536,7 +555,7 @@ struct se_cmd {
 	struct se_lun		*se_lun;
 	/* Only used for internal passthrough and legacy TCM fabric modules */
 	struct se_session	*se_sess;
-	struct se_tmr_req	*se_tmr_req;
+	struct se_tmr_req	se_tmr_req;
 	struct list_head	se_queue_node;
 	struct list_head	se_cmd_list;
 	struct completion	cmd_wait_comp;
@@ -585,24 +604,6 @@ struct se_cmd {
 
 };
 
-struct se_tmr_req {
-	/* Task Management function to be performed */
-	u8			function;
-	/* Task Management response to send */
-	u8			response;
-	int			call_transport;
-	/* Reference to ITT that Task Mgmt should be performed */
-	u32			ref_task_tag;
-	/* 64-bit encoded SAM LUN from $FABRIC_MOD TMR header */
-	u64			ref_task_lun;
-	void 			*fabric_tmr_ptr;
-	struct se_cmd		*task_cmd;
-	struct se_cmd		*ref_cmd;
-	struct se_device	*tmr_dev;
-	struct se_lun		*tmr_lun;
-	struct list_head	tmr_list;
-};
-
 struct se_ua {
 	u8			ua_asc;
 	u8			ua_ascq;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 3dac347..9684f32 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -144,7 +144,7 @@ void	target_wait_for_sess_cmds(struct se_session *, int);
 
 int	core_alua_check_nonop_delay(struct se_cmd *);
 
-struct se_tmr_req *core_tmr_alloc_req(struct se_cmd *, void *, u8, gfp_t);
+void	core_tmr_req_init(struct se_cmd *, void *, u8);
 void	core_tmr_release_req(struct se_tmr_req *);
 int	transport_generic_handle_tmr(struct se_cmd *);
 int	transport_lookup_tmr_lun(struct se_cmd *, u32);
-- 
1.7.1

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Christoph Hellwig]

On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
> This saves all fabrics from calling core_tmr_alloc_req() and
> having to check the result. The downside is se_cmd gets bigger for all
> requests, but hopefully later patches will reduce it.

Without patches to void the overhead it's not acceptable. Fortunately
it should be doable fairly simply by using an union for command vs
TMR fields.

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Andy Grover]

On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
> On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
>> This saves all fabrics from calling core_tmr_alloc_req() and
>> having to check the result. The downside is se_cmd gets bigger for all
>> requests, but hopefully later patches will reduce it.
> 
> Without patches to void the overhead it's not acceptable. Fortunately
> it should be doable fairly simply by using an union for command vs
> TMR fields.

This was my thought too. We should be able to move cmd variables into a
union w/tmr struct very soon, with a low risk of introducing bugs.

-- Andy

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Nicholas A. Bellinger]

On Fri, 2012-01-20 at 09:44 -0800, Andy Grover wrote:
> On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
> > On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
> >> This saves all fabrics from calling core_tmr_alloc_req() and
> >> having to check the result. The downside is se_cmd gets bigger for all
> >> requests, but hopefully later patches will reduce it.
> > 
> > Without patches to void the overhead it's not acceptable. Fortunately
> > it should be doable fairly simply by using an union for command vs
> > TMR fields.
> 
> This was my thought too. We should be able to move cmd variables into a
> union w/tmr struct very soon, with a low risk of introducing bugs.
> 

Hi Andy,

Ping on this..?  I've been merging patches from lio-core into
target-pending/for-next the past week, and i've stopped ahead of this
patch to inline se_tmr_req into se_cmd due of the extra overhead in
question here..

I'd really like to get this resolved in v3.4 for-next, so would you mind
taking care of the lio-core conversion to union for command vs
TMR fields so this can be squashed into for-next..?

Thanks,

--nab

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Andy Grover]

On 02/15/2012 04:05 PM, Nicholas A. Bellinger wrote:
> On Fri, 2012-01-20 at 09:44 -0800, Andy Grover wrote:
>> On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
>>> On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
>>>> This saves all fabrics from calling core_tmr_alloc_req() and
>>>> having to check the result. The downside is se_cmd gets bigger for all
>>>> requests, but hopefully later patches will reduce it.
>>>
>>> Without patches to void the overhead it's not acceptable. Fortunately
>>> it should be doable fairly simply by using an union for command vs
>>> TMR fields.
>>
>> This was my thought too. We should be able to move cmd variables into a
>> union w/tmr struct very soon, with a low risk of introducing bugs.
>>
> 
> Hi Andy,
> 
> Ping on this..?  I've been merging patches from lio-core into
> target-pending/for-next the past week, and i've stopped ahead of this
> patch to inline se_tmr_req into se_cmd due of the extra overhead in
> question here..
> 
> I'd really like to get this resolved in v3.4 for-next, so would you mind
> taking care of the lio-core conversion to union for command vs
> TMR fields so this can be squashed into for-next..?

Hi Nick,

Haven't had a chance to make further progress. You can drop it and I'll
resubmit it later with the supplementary work.

Regards -- Andy

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Nicholas A. Bellinger]

On Wed, 2012-02-15 at 22:30 -0800, Andy Grover wrote:
> On 02/15/2012 04:05 PM, Nicholas A. Bellinger wrote:
> > On Fri, 2012-01-20 at 09:44 -0800, Andy Grover wrote:
> >> On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
> >>> On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
> >>>> This saves all fabrics from calling core_tmr_alloc_req() and
> >>>> having to check the result. The downside is se_cmd gets bigger for all
> >>>> requests, but hopefully later patches will reduce it.
> >>>
> >>> Without patches to void the overhead it's not acceptable. Fortunately
> >>> it should be doable fairly simply by using an union for command vs
> >>> TMR fields.
> >>
> >> This was my thought too. We should be able to move cmd variables into a
> >> union w/tmr struct very soon, with a low risk of introducing bugs.
> >>
> > 
> > Hi Andy,
> > 
> > Ping on this..?  I've been merging patches from lio-core into
> > target-pending/for-next the past week, and i've stopped ahead of this
> > patch to inline se_tmr_req into se_cmd due of the extra overhead in
> > question here..
> > 
> > I'd really like to get this resolved in v3.4 for-next, so would you mind
> > taking care of the lio-core conversion to union for command vs
> > TMR fields so this can be squashed into for-next..?
> 
> Hi Nick,
> 
> Haven't had a chance to make further progress. You can drop it and I'll
> resubmit it later with the supplementary work.
> 

Hi Andy,

Ok, I'll go ahead and re-add allocation of se_tmr_req into lio-core, and
drop this patch from my for-next queue for now.

--nab

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Christoph Hellwig]

On Fri, Jan 20, 2012 at 09:44:43AM -0800, Andy Grover wrote:
> On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
> > On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
> >> This saves all fabrics from calling core_tmr_alloc_req() and
> >> having to check the result. The downside is se_cmd gets bigger for all
> >> requests, but hopefully later patches will reduce it.
> > 
> > Without patches to void the overhead it's not acceptable. Fortunately
> > it should be doable fairly simply by using an union for command vs
> > TMR fields.
> 
> This was my thought too. We should be able to move cmd variables into a
> union w/tmr struct very soon, with a low risk of introducing bugs.

Any plans on going back to this?

Re: [PATCH 07/13] target: Inline struct se_tmr_req into se_cmd

[Andy Grover]

On 03/26/2012 02:25 AM, Christoph Hellwig wrote:
> On Fri, Jan 20, 2012 at 09:44:43AM -0800, Andy Grover wrote:
>> On 01/20/2012 08:12 AM, Christoph Hellwig wrote:
>>> On Thu, Jan 19, 2012 at 01:39:17PM -0800, Andy Grover wrote:
>>>> This saves all fabrics from calling core_tmr_alloc_req() and
>>>> having to check the result. The downside is se_cmd gets bigger for all
>>>> requests, but hopefully later patches will reduce it.
>>>
>>> Without patches to void the overhead it's not acceptable. Fortunately
>>> it should be doable fairly simply by using an union for command vs
>>> TMR fields.
>>
>> This was my thought too. We should be able to move cmd variables into a
>> union w/tmr struct very soon, with a low risk of introducing bugs.
> 
> Any plans on going back to this?

I'm working on a cleanup patch series that keeps getting bigger, and
this will either end up in it, or I'll get to it right afterwards.

-- Andy

[PATCH 08/13] target/fc: Move core->fc code conversion earlier in ft_send_tm()

[Andy Grover]

No dependencies on rest of code, let's get tm_func set asap.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_cmd.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index f7dad73..bd69ce9 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -357,11 +357,6 @@ static void ft_send_tm(struct ft_cmd *cmd)
 	struct ft_sess *sess;
 	u8 tm_func;
 
-	transport_init_se_cmd(&cmd->se_cmd, &ft_configfs->tf_ops,
-			cmd->sess->se_sess, 0, DMA_NONE, 0,
-			&cmd->ft_sense_buffer[0]);
-	target_get_sess_cmd(cmd->sess->se_sess, &cmd->se_cmd, false);
-
 	fcp = fc_frame_payload_get(cmd->req_frame, sizeof(*fcp));
 
 	switch (fcp->fc_tm_flags) {
@@ -390,6 +385,11 @@ static void ft_send_tm(struct ft_cmd *cmd)
 		return;
 	}
 
+	transport_init_se_cmd(&cmd->se_cmd, &ft_configfs->tf_ops,
+			cmd->sess->se_sess, 0, DMA_NONE, 0,
+			&cmd->ft_sense_buffer[0]);
+	target_get_sess_cmd(cmd->sess->se_sess, &cmd->se_cmd, false);
+
 	pr_debug("alloc tm cmd fn %d\n", tm_func);
 	core_tmr_req_init(&cmd->se_cmd, cmd, tm_func);
 
-- 
1.7.1

[PATCH 09/13] target/fc: Call lookup_tmr_lun() for all TM types

[Andy Grover]

Don't see a reason to differentiate.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_cmd.c |   38 +++++++++++---------------------------
 1 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index bd69ce9..e784514 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -354,11 +354,10 @@ static void ft_send_resp_code_and_free(struct ft_cmd *cmd,
 static void ft_send_tm(struct ft_cmd *cmd)
 {
 	struct fcp_cmnd *fcp;
-	struct ft_sess *sess;
 	u8 tm_func;
+	int ret;
 
 	fcp = fc_frame_payload_get(cmd->req_frame, sizeof(*fcp));
-
 	switch (fcp->fc_tm_flags) {
 	case FCP_TMF_LUN_RESET:
 		tm_func = TMR_LUN_RESET;
@@ -393,33 +392,18 @@ static void ft_send_tm(struct ft_cmd *cmd)
 	pr_debug("alloc tm cmd fn %d\n", tm_func);
 	core_tmr_req_init(&cmd->se_cmd, cmd, tm_func);
 
-	switch (fcp->fc_tm_flags) {
-	case FCP_TMF_LUN_RESET:
-		if (transport_lookup_tmr_lun(&cmd->se_cmd, scsilun_to_int(&fcp->fc_lun)) < 0) {
-			/*
-			 * Make sure to clean up newly allocated TMR request
-			 * since "unable to  handle TMR request because failed
-			 * to get to LUN"
-			 */
-			pr_debug("Failed to get LUN for TMR func %d, "
-				  "se_cmd %p, unpacked_lun %d\n",
-				  tm_func, &cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
-			ft_dump_cmd(cmd, __func__);
-			sess = cmd->sess;
-			transport_send_check_condition_and_sense(&cmd->se_cmd,
-				cmd->se_cmd.scsi_sense_reason, 0);
-			ft_sess_put(sess);
-			return;
-		}
-		break;
-	case FCP_TMF_TGT_RESET:
-	case FCP_TMF_CLR_TASK_SET:
-	case FCP_TMF_ABT_TASK_SET:
-	case FCP_TMF_CLR_ACA:
-		break;
-	default:
+	ret = transport_lookup_tmr_lun(&cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
+	if (ret < 0) {
+		pr_debug("Failed to get LUN for TMR func %d, "
+			 "se_cmd %p, unpacked_lun %d\n",
+			 tm_func, &cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
+		ft_dump_cmd(cmd, __func__);
+		transport_send_check_condition_and_sense(&cmd->se_cmd,
+			cmd->se_cmd.scsi_sense_reason, 0);
+		ft_sess_put(cmd->sess);
 		return;
 	}
+
 	transport_generic_handle_tmr(&cmd->se_cmd);
 }
 
-- 
1.7.1

[PATCH 10/13] target/fc: Use transport_generic_free_cmd for ft_sess_put in ft_send_tm

[Andy Grover]

transport_generic_free_cmd will end up calling ft_sess_put, so it should
work just the same.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_cmd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index e784514..da73d0a 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -400,7 +400,7 @@ static void ft_send_tm(struct ft_cmd *cmd)
 		ft_dump_cmd(cmd, __func__);
 		transport_send_check_condition_and_sense(&cmd->se_cmd,
 			cmd->se_cmd.scsi_sense_reason, 0);
-		ft_sess_put(cmd->sess);
+		transport_generic_free_cmd(&cmd->se_cmd, 0);
 		return;
 	}
 
-- 
1.7.1

[PATCH 11/13] target: Add target_submit_tmr helper function

[Andy Grover]

Similar to target_submit_cmd, this function lets fabrics call one function
(albeit with a lot of parameters) instead of 3 or more.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_transport.c |   42 ++++++++++++++++++++++++++++++++
 include/target/target_core_fabric.h    |    3 ++
 2 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 314567a..15faca4 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1691,6 +1691,48 @@ out_check_cond:
 }
 EXPORT_SYMBOL(target_submit_cmd);
 
+/**
+ * target_submit_tmr - lookup unpacked lun and submit uninitialized se_cmd
+ *                     for TMR CDBs
+ *
+ * @se_cmd: command descriptor to submit
+ * @se_sess: associated se_sess for endpoint
+ * @sense: pointer to SCSI sense buffer
+ * @unpacked_lun: unpacked LUN to reference for struct se_lun
+ * @fabric_context: fabric context for TMR req
+ * @tm_type: Type of TM request
+ *
+ * Callable from all contexts.
+ **/
+
+void target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
+		unsigned char *sense, u32 unpacked_lun,
+		void *fabric_tmr_ptr, unsigned char tm_type, int flags)
+{
+	struct se_portal_group *se_tpg;	
+	int ret;
+
+	se_tpg = se_sess->se_tpg;
+	BUG_ON(!se_tpg);
+
+	transport_init_se_cmd(se_cmd, se_tpg->se_tpg_tfo, se_sess,
+			      0, DMA_NONE, MSG_SIMPLE_TAG, sense);
+
+	/* See target_submit_cmd for commentary */
+	target_get_sess_cmd(se_sess, se_cmd, (flags & TARGET_SCF_ACK_KREF));
+
+	core_tmr_req_init(se_cmd, fabric_tmr_ptr, tm_type);
+
+	ret = transport_lookup_tmr_lun(se_cmd, unpacked_lun);
+	if (ret) {
+		transport_send_check_condition_and_sense(se_cmd,
+			se_cmd->scsi_sense_reason, 0);
+		transport_generic_free_cmd(se_cmd, 0);
+	}
+	transport_generic_handle_tmr(se_cmd);
+}
+EXPORT_SYMBOL(target_submit_tmr);
+
 /*
  * Used by fabric module frontends defining a TFO->new_cmd_map() caller
  * to  queue up a newly setup se_cmd w/ TRANSPORT_NEW_CMD_MAP in order to
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 9684f32..7d0e2e0 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -121,6 +121,9 @@ int	transport_generic_allocate_tasks(struct se_cmd *, unsigned char *);
 int	transport_handle_cdb_direct(struct se_cmd *);
 int	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
 		unsigned char *, u32, u32, int, int, int);
+void	target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
+		unsigned char *sense, u32 unpacked_lun,
+		void *fabric_tmr_ptr, unsigned char tm_type, int flags);
 int	transport_generic_handle_cdb_map(struct se_cmd *);
 int	transport_generic_handle_data(struct se_cmd *);
 int	transport_generic_map_mem_to_cmd(struct se_cmd *cmd,
-- 
1.7.1

[PATCH 12/13] target/fc: Use target_submit_tmr()

[Andy Grover]

Make use of the new helper function to submit a TMR.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_cmd.c |   25 +++----------------------
 1 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index da73d0a..08a10e1 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -355,7 +355,6 @@ static void ft_send_tm(struct ft_cmd *cmd)
 {
 	struct fcp_cmnd *fcp;
 	u8 tm_func;
-	int ret;
 
 	fcp = fc_frame_payload_get(cmd->req_frame, sizeof(*fcp));
 	switch (fcp->fc_tm_flags) {
@@ -384,27 +383,9 @@ static void ft_send_tm(struct ft_cmd *cmd)
 		return;
 	}
 
-	transport_init_se_cmd(&cmd->se_cmd, &ft_configfs->tf_ops,
-			cmd->sess->se_sess, 0, DMA_NONE, 0,
-			&cmd->ft_sense_buffer[0]);
-	target_get_sess_cmd(cmd->sess->se_sess, &cmd->se_cmd, false);
-
-	pr_debug("alloc tm cmd fn %d\n", tm_func);
-	core_tmr_req_init(&cmd->se_cmd, cmd, tm_func);
-
-	ret = transport_lookup_tmr_lun(&cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
-	if (ret < 0) {
-		pr_debug("Failed to get LUN for TMR func %d, "
-			 "se_cmd %p, unpacked_lun %d\n",
-			 tm_func, &cmd->se_cmd, scsilun_to_int(&fcp->fc_lun));
-		ft_dump_cmd(cmd, __func__);
-		transport_send_check_condition_and_sense(&cmd->se_cmd,
-			cmd->se_cmd.scsi_sense_reason, 0);
-		transport_generic_free_cmd(&cmd->se_cmd, 0);
-		return;
-	}
-
-	transport_generic_handle_tmr(&cmd->se_cmd);
+	target_submit_tmr(&cmd->se_cmd, cmd->sess->se_sess,
+		&cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
+			  cmd, tm_func, 0);
 }
 
 /*
-- 
1.7.1

[PATCH 13/13] target: Change target_submit_cmd() to return void

[Andy Grover]

Retval not very useful, and may even be harmful. Once submitted, fabrics
should expect a sense error if anything goes wrong. All fabrics checking
of this retval are useless or broken:

fc checks it just to emit more debug output.
ib_srpt trickles retval up, then it is ignored.
qla2xxx trickles it up, which then causes a bug because the abort goto
in qla_target.c thinks cmd hasn't been sent to target.

Just returning nothing is best.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c  |    5 +----
 drivers/scsi/qla2xxx/tcm_qla2xxx.c     |    7 ++-----
 drivers/target/target_core_transport.c |    5 ++---
 drivers/target/tcm_fc/tfc_cmd.c        |    9 ++-------
 include/target/target_core_fabric.h    |    2 +-
 5 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5ec4f9d..57dbedc 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1692,12 +1692,9 @@ static int srpt_handle_cmd(struct srpt_rdma_ch *ch,
 	}
 	unpacked_lun = srpt_unpack_lun((uint8_t *)&srp_cmd->lun,
 				       sizeof(srp_cmd->lun));
-	ret = target_submit_cmd(cmd, ch->sess, srp_cmd->cdb,
+	target_submit_cmd(cmd, ch->sess, srp_cmd->cdb,
 			&send_ioctx->sense_data[0], unpacked_lun, data_len,
 			MSG_SIMPLE_TAG, dir, TARGET_SCF_ACK_KREF);
-	if (ret != 0)
-		return -1;
-
 	return 0;
 
 send_sense:
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 7132c1c..83e5df4 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -597,7 +597,7 @@ int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
 	struct se_cmd *se_cmd = &cmd->se_cmd;
 	struct se_session *se_sess;
 	struct qla_tgt_sess *sess;
-	int rc, flags = TARGET_SCF_ACK_KREF;
+	int flags = TARGET_SCF_ACK_KREF;
 
 	if (bidi)
 		flags |= TARGET_SCF_BIDI_OP;
@@ -614,12 +614,9 @@ int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
 		return -EINVAL;
 	}
 
-	rc = target_submit_cmd(se_cmd, se_sess, cdb, &cmd->sense_buffer[0],
+	target_submit_cmd(se_cmd, se_sess, cdb, &cmd->sense_buffer[0],
 				cmd->unpacked_lun, data_length, fcp_task_attr,
 				data_dir, flags);
-	if (rc != 0)
-		return -EINVAL;
-
 	return 0;
 }
 
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 15faca4..cfc4e02 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1633,7 +1633,7 @@ EXPORT_SYMBOL(transport_handle_cdb_direct);
  * This may only be called from process context, and also currently
  * assumes internal allocation of fabric payload buffer by target-core.
  **/
-int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
+void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 		unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
 		u32 data_length, int task_attr, int data_dir, int flags)
 {
@@ -1682,12 +1682,11 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 	 * when fabric has filled the incoming buffer.
 	 */
 	transport_handle_cdb_direct(se_cmd);
-	return 0;
+	return;
 
 out_check_cond:
 	transport_send_check_condition_and_sense(se_cmd,
 				se_cmd->scsi_sense_reason, 0);
-	return 0;
 }
 EXPORT_SYMBOL(target_submit_cmd);
 
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 08a10e1..938240b 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -491,7 +491,6 @@ static void ft_send_work(struct work_struct *work)
 	struct fcp_cmnd *fcp;
 	int data_dir = 0;
 	int task_attr;
-	int ret;
 
 	fcp = fc_frame_payload_get(cmd->req_frame, sizeof(*fcp));
 	if (!fcp)
@@ -544,14 +543,10 @@ static void ft_send_work(struct work_struct *work)
  	 * Use a single se_cmd->cmd_kref as we expect to release se_cmd
  	 * directly from ft_check_stop_free callback in response path.
  	 */
-	ret = target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, fcp->fc_cdb,
+	target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, fcp->fc_cdb,
 				&cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
 				ntohl(fcp->fc_dl), task_attr, data_dir, 0);
-	pr_debug("r_ctl %x alloc target_submit_cmd %d\n", fh->fh_r_ctl, ret);
-	if (ret < 0) {
-		ft_dump_cmd(cmd, __func__);
-		return;
-	}
+	pr_debug("r_ctl %x alloc target_submit_cmd\n", fh->fh_r_ctl);
 	return;
 
 err:
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 7d0e2e0..cfd908f 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -119,7 +119,7 @@ void	transport_init_se_cmd(struct se_cmd *, struct target_core_fabric_ops *,
 int	transport_lookup_cmd_lun(struct se_cmd *, u32);
 int	transport_generic_allocate_tasks(struct se_cmd *, unsigned char *);
 int	transport_handle_cdb_direct(struct se_cmd *);
-int	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
+void	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
 		unsigned char *, u32, u32, int, int, int);
 void	target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
 		unsigned char *sense, u32 unpacked_lun,
-- 
1.7.1

Re: [PATCH 13/13] target: Change target_submit_cmd() to return void

[Nicholas A. Bellinger]

On Thu, 2012-01-19 at 13:39 -0800, Andy Grover wrote:
> Retval not very useful, and may even be harmful. Once submitted, fabrics
> should expect a sense error if anything goes wrong. All fabrics checking
> of this retval are useless or broken:
> 
> fc checks it just to emit more debug output.
> ib_srpt trickles retval up, then it is ignored.
> qla2xxx trickles it up, which then causes a bug because the abort goto
> in qla_target.c thinks cmd hasn't been sent to target.
> 
> Just returning nothing is best.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>
> ---
>  drivers/infiniband/ulp/srpt/ib_srpt.c  |    5 +----
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c     |    7 ++-----
>  drivers/target/target_core_transport.c |    5 ++---
>  drivers/target/tcm_fc/tfc_cmd.c        |    9 ++-------
>  include/target/target_core_fabric.h    |    2 +-
>  5 files changed, 8 insertions(+), 20 deletions(-)
> 

<SNIP>

> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 15faca4..cfc4e02 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -1633,7 +1633,7 @@ EXPORT_SYMBOL(transport_handle_cdb_direct);
>   * This may only be called from process context, and also currently
>   * assumes internal allocation of fabric payload buffer by target-core.
>   **/
> -int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
> +void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>  		unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
>  		u32 data_length, int task_attr, int data_dir, int flags)
>  {
> @@ -1682,12 +1682,11 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>  	 * when fabric has filled the incoming buffer.
>  	 */
>  	transport_handle_cdb_direct(se_cmd);
> -	return 0;
> +	return;
>  
>  out_check_cond:
>  	transport_send_check_condition_and_sense(se_cmd,
>  				se_cmd->scsi_sense_reason, 0);
> -	return 0;
>  }
>  EXPORT_SYMBOL(target_submit_cmd);
>  

Your patch is fine here..

However my original usage of transport_send_check_condition_and_sense()
is wrong wrt target_submit_cmd() failures.  The cmd_kref will have
already been taken by target_get_sess_cmd(), so this needs to be using
transport_generic_request_failure() around sense_check_condition to
handle proper TFO->check_stop_free() kref_puts + response queue_full
cases.

So I'm looking into addressing this breakage in lio-core now, and will
post a patch for 3.3-rc-fixes soon after I can verify the fix with
qla2xxx + ib_srpt failures.

--nab

Re: [PATCH 13/13] target: Change target_submit_cmd() to return void

[Nicholas A. Bellinger]

On Thu, 2012-01-19 at 13:39 -0800, Andy Grover wrote:
> Retval not very useful, and may even be harmful. Once submitted, fabrics
> should expect a sense error if anything goes wrong. All fabrics checking
> of this retval are useless or broken:
> 
> fc checks it just to emit more debug output.
> ib_srpt trickles retval up, then it is ignored.
> qla2xxx trickles it up, which then causes a bug because the abort goto
> in qla_target.c thinks cmd hasn't been sent to target.
> 
> Just returning nothing is best.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>
> ---
>  drivers/infiniband/ulp/srpt/ib_srpt.c  |    5 +----
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c     |    7 ++-----
>  drivers/target/target_core_transport.c |    5 ++---
>  drivers/target/tcm_fc/tfc_cmd.c        |    9 ++-------
>  include/target/target_core_fabric.h    |    2 +-
>  5 files changed, 8 insertions(+), 20 deletions(-)
> 

Just a heads-up for Sebastian..

I've merged Andy's patch with the following change to usb-gadget to also
ignore the return from target_submit_cmd() and let target-core invoke
the proper response callbacks, et al with the recently posted kref_put
leak bugfix..

AFAICT this should work as expected with usb-gadget, but please let me
know if you run into problems.  ;)

Thanks,

--nab

diff --git a/drivers/target/usb-gadget/fabric.c b/drivers/target/usb-gadget/fabric.c
index f56319a..7f432c2 100644
--- a/drivers/target/usb-gadget/fabric.c
+++ b/drivers/target/usb-gadget/fabric.c
@@ -181,7 +181,6 @@ static void usbg_cmd_work(struct work_struct *work)
        struct se_cmd *se_cmd;
        struct tcm_usbg_nexus *tv_nexus;
        struct usbg_tpg *tpg;
-       int ret;
        int dir;
 
        se_cmd = &cmd->se_cmd;
@@ -199,11 +198,9 @@ static void usbg_cmd_work(struct work_struct *work)
                return;
        }
 
-       ret = target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess,
+       target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess,
                        cmd->cmd_buf, cmd->sense_iu.sense, cmd->unpacked_lun,
                        0, cmd->prio_attr, dir, TARGET_SCF_UNKNOWN_SIZE);
-       if (ret)
-               kref_put(&cmd->ref, usbg_cmd_release);
 }
 
 int usbg_submit_command(struct f_uas *fu,

Re: [PATCH 0/13] Misc patches leading to target_submit_tmr

[Nicholas A. Bellinger]

On Thu, 2012-01-19 at 13:39 -0800, Andy Grover wrote:
> Hi nab, Kiran, and all,
> 
> Here are some cleanup patches to target, scsi, and tcm_fc. The biggest
> change is adding a target_submit_tmr to match the new target_submit_cmd
> helper function. I've only changed tcm_fc to use it so far, if it looks ok
> I will change over the other fabrics.
> 

Nice work to get target_submit_tmr() added here..

> I'm especially looking for review of the error handling changes in the
> last few patches. Does transport_generic_free_cmd end up doing the
> same thing as ft_sess_put, for example?
> 
> Thanks -- Regards -- Andy
> 

After an initial review I think these look reasonable to merge into
lio-core for testing.

Kiran, could you have a look the tcm_fc pieces and us know if you have
concerns..?

Thanks Andy!

--nab

> The following changes since commit 7acd456a2fdae22d0f43db60f689ba3c57b69838:
> 
>   target: Fail INQUIRY commands with EVPD==0 but PAGE CODE!=0 (2012-01-17 22:54:29 -0800)
> 
> are available in the git repository at:
>   git://fedorapeople.org/home/fedora/grover/public_git/linux-2.6.git for-nab
> 
> Andy Grover (13):
>       scsi: Use struct scsi_lun in fc/fcp.h
>       target: fix comment typos
>       target: Remove unused struct se_queue_req
>       target/iscsi: Remove unneeded wrapper functions
>       target/fc: Simplify ft_send_work for tmr path
>       target/fc: Remove cmd->cdb data member
>       target: Inline struct se_tmr_req into se_cmd
>       target/fc: Move core->fc code conversion earlier in ft_send_tm()
>       target/fc: Call lookup_tmr_lun() for all TM types
>       target/fc: Use transport_generic_free_cmd for ft_sess_put in ft_send_tm
>       target: Add target_submit_tmr helper function
>       target/fc: Use target_submit_tmr()
>       target: Change target_submit_cmd() to return void
> 
>  drivers/infiniband/ulp/srpt/ib_srpt.c      |   18 +---
>  drivers/scsi/bnx2fc/bnx2fc_io.c            |    4 +-
>  drivers/scsi/libfc/fc_fcp.c                |    5 +-
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c         |   22 ++---
>  drivers/target/iscsi/iscsi_target.c        |   12 +-
>  drivers/target/iscsi/iscsi_target_device.c |   19 ----
>  drivers/target/iscsi/iscsi_target_device.h |    2 -
>  drivers/target/iscsi/iscsi_target_tmr.c    |    6 +-
>  drivers/target/iscsi/iscsi_target_util.c   |    8 +-
>  drivers/target/loopback/tcm_loop.c         |   14 +--
>  drivers/target/target_core_device.c        |    2 +-
>  drivers/target/target_core_internal.h      |    2 -
>  drivers/target/target_core_tmr.c           |   20 +---
>  drivers/target/target_core_transport.c     |   77 +++++++++++-----
>  drivers/target/tcm_fc/tcm_fc.h             |    2 -
>  drivers/target/tcm_fc/tfc_cmd.c            |  142 ++++++++--------------------
>  include/scsi/fc/fc_fcp.h                   |    6 +-
>  include/target/target_core_base.h          |   47 ++++-----
>  include/target/target_core_fabric.h        |    7 +-
>  19 files changed, 164 insertions(+), 251 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html