All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu()
@ 2009-02-11  3:01 Karen Xie
       [not found] ` <200902110301.n1B31q5M002101-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
  2009-02-11 19:57 ` Mike Christie
  0 siblings, 2 replies; 3+ messages in thread
From: Karen Xie @ 2009-02-11  3:01 UTC (permalink / raw)
  To: open-iscsi-/JYPxA39Uh5TLH3MbocFFw, linux-scsi-u79uwXL29TY76Z2rM5mHXA
  Cc: michaelc-hcNo3dDEHLuVc3sceRu5cw, kxie-ut6Up61K2wZBDgjK7y7TUQ,
	James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk


[PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu().

From: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>

Hi, Mike,

I looked through libiscsi.c, libiscsi_tcp.c and iscsi_tcp.c. It does seem to be a little messy to merge the two functions. Especially the BHS is constructed after pdu_alloc(), and iscsi_tcp uses the BHS fields in init_pdu(). 

So I only added the offset and count as additional parameters to alloc_pdu(). So that the pdu payload is known at the time of pdu memory allocation.

Thanks,
Karen

Signed-off-by: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---

 drivers/scsi/iscsi_tcp.c            |    3 +
 drivers/scsi/libiscsi.c             |   93 +++++++++++++++++------------------
 drivers/scsi/libiscsi_tcp.c         |    6 ++
 include/scsi/libiscsi.h             |   12 +++++
 include/scsi/scsi_transport_iscsi.h |    3 +
 5 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 23808df..7fe0c68 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -458,7 +458,8 @@ static int iscsi_sw_tcp_pdu_init(struct iscsi_task *task,
 	return 0;
 }
 
-static int iscsi_sw_tcp_pdu_alloc(struct iscsi_task *task, uint8_t opcode)
+static int iscsi_sw_tcp_pdu_alloc(struct iscsi_task *task, uint8_t opcode,
+				 unsigned int offset, unsigned int count)
 {
 	struct iscsi_tcp_task *tcp_task = task->dd_data;
 
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 257c241..81956ac 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -104,7 +104,6 @@ void iscsi_prep_data_out_pdu(struct iscsi_task *task, struct iscsi_r2t_info *r2t
 			   struct iscsi_data *hdr)
 {
 	struct iscsi_conn *conn = task->conn;
-	unsigned int left = r2t->data_length - r2t->sent;
 
 	task->hdr_len = sizeof(struct iscsi_data);
 
@@ -117,15 +116,11 @@ void iscsi_prep_data_out_pdu(struct iscsi_task *task, struct iscsi_r2t_info *r2t
 	hdr->itt = task->hdr_itt;
 	hdr->exp_statsn = r2t->exp_statsn;
 	hdr->offset = cpu_to_be32(r2t->data_offset + r2t->sent);
-	if (left > conn->max_xmit_dlength) {
-		hton24(hdr->dlength, conn->max_xmit_dlength);
-		r2t->data_count = conn->max_xmit_dlength;
-		hdr->flags = 0;
-	} else {
-		hton24(hdr->dlength, left);
-		r2t->data_count = left;
+	hton24(hdr->dlength, r2t->data_count);
+	if (r2t->data_length == (r2t->sent + r2t->data_count))
 		hdr->flags = ISCSI_FLAG_CMD_FINAL;
-	}
+	else
+		hdr->flags = 0;
 	conn->dataout_pdus_cnt++;
 }
 EXPORT_SYMBOL_GPL(iscsi_prep_data_out_pdu);
@@ -225,7 +220,45 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
 	itt_t itt;
 	int rc;
 
-	rc = conn->session->tt->alloc_pdu(task, ISCSI_OP_SCSI_CMD);
+	if (sc->sc_data_direction == DMA_TO_DEVICE) {
+		unsigned out_len = scsi_out(sc)->length;
+		struct iscsi_r2t_info *r2t = &task->unsol_r2t;
+
+		/*
+		 * Write counters:
+		 *
+		 *	imm_count	bytes to be sent right after
+		 *			SCSI PDU Header
+		 *
+		 *	unsol_count	bytes(as Data-Out) to be sent
+		 *			without	R2T ack right after
+		 *			immediate data
+		 *
+		 *	r2t data_length bytes to be sent via R2T ack's
+		 *
+		 *      pad_count       bytes to be sent as zero-padding
+		 */
+		if (session->imm_data_en) {
+			if (out_len >= session->first_burst)
+				task->imm_count = min(session->first_burst,
+							conn->max_xmit_dlength);
+			else
+				task->imm_count = min(out_len,
+							conn->max_xmit_dlength);
+		}
+
+		memset(r2t, 0, sizeof(*r2t));
+		if (!session->initial_r2t_en) {
+			r2t->data_length = min(session->first_burst, out_len) -
+					       task->imm_count;
+			r2t->data_offset = task->imm_count;
+			r2t->ttt = cpu_to_be32(ISCSI_RESERVED_TAG);
+			r2t->exp_statsn = cpu_to_be32(conn->exp_statsn);
+		}
+	}
+
+	rc = conn->session->tt->alloc_pdu(task, ISCSI_OP_SCSI_CMD, 0,
+					task->imm_count);
 	if (rc)
 		return rc;
 	hdr = (struct iscsi_cmd *) task->hdr;
@@ -259,7 +292,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
 	}
 	memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
-	task->imm_count = 0;
 	if (scsi_bidi_cmnd(sc)) {
 		hdr->flags |= ISCSI_FLAG_CMD_READ;
 		rc = iscsi_prep_bidi_ahs(task);
@@ -267,46 +299,13 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
 			return rc;
 	}
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
-		unsigned out_len = scsi_out(sc)->length;
-		struct iscsi_r2t_info *r2t = &task->unsol_r2t;
-
-		hdr->data_length = cpu_to_be32(out_len);
+		hdr->data_length = cpu_to_be32(scsi_out(sc)->length);
 		hdr->flags |= ISCSI_FLAG_CMD_WRITE;
-		/*
-		 * Write counters:
-		 *
-		 *	imm_count	bytes to be sent right after
-		 *			SCSI PDU Header
-		 *
-		 *	unsol_count	bytes(as Data-Out) to be sent
-		 *			without	R2T ack right after
-		 *			immediate data
-		 *
-		 *	r2t data_length bytes to be sent via R2T ack's
-		 *
-		 *      pad_count       bytes to be sent as zero-padding
-		 */
-		memset(r2t, 0, sizeof(*r2t));
-
-		if (session->imm_data_en) {
-			if (out_len >= session->first_burst)
-				task->imm_count = min(session->first_burst,
-							conn->max_xmit_dlength);
-			else
-				task->imm_count = min(out_len,
-							conn->max_xmit_dlength);
+		if (task->imm_count) {
 			hton24(hdr->dlength, task->imm_count);
 		} else
 			zero_data(hdr->dlength);
 
-		if (!session->initial_r2t_en) {
-			r2t->data_length = min(session->first_burst, out_len) -
-					       task->imm_count;
-			r2t->data_offset = task->imm_count;
-			r2t->ttt = cpu_to_be32(ISCSI_RESERVED_TAG);
-			r2t->exp_statsn = cpu_to_be32(conn->exp_statsn);
-		}
-
 		if (!task->unsol_r2t.data_length)
 			/* No unsolicit Data-Out's */
 			hdr->flags |= ISCSI_FLAG_CMD_FINAL;
@@ -532,7 +531,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
 	} else
 		task->data_count = 0;
 
-	if (conn->session->tt->alloc_pdu(task, hdr->opcode)) {
+	if (conn->session->tt->alloc_pdu(task, hdr->opcode, 0, data_size)) {
 		iscsi_conn_printk(KERN_ERR, conn, "Could not allocate "
 				 "pdu for mgmt task.\n");
 		goto requeue_task;
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index e7705d3..23fad1b 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -1023,7 +1023,11 @@ flush:
 		return 0;
 	}
 
-	rc = conn->session->tt->alloc_pdu(task, ISCSI_OP_SCSI_DATA_OUT);
+	iscsi_calc_data_out_payload(task, r2t);
+
+	rc = conn->session->tt->alloc_pdu(task, ISCSI_OP_SCSI_DATA_OUT,
+					r2t->data_offset + r2t->sent,
+					r2t->data_count);
 	if (rc)
 		return rc;
 	iscsi_prep_data_out_pdu(task, r2t, (struct iscsi_data *) task->hdr);
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 7360e19..2097ee5 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -395,6 +395,18 @@ extern void iscsi_suspend_tx(struct iscsi_conn *conn);
 /*
  * pdu and task processing
  */
+static inline void iscsi_calc_data_out_payload(struct iscsi_task *task,
+						struct iscsi_r2t_info *r2t)
+{
+	struct iscsi_conn *conn = task->conn;
+	unsigned int left = r2t->data_length - r2t->sent;
+
+	if (left > conn->max_xmit_dlength)
+		r2t->data_count = conn->max_xmit_dlength;
+	else
+		r2t->data_count = left;
+}
+
 extern void iscsi_update_cmdsn(struct iscsi_session *, struct iscsi_nopin *);
 extern void iscsi_prep_data_out_pdu(struct iscsi_task *task,
 				    struct iscsi_r2t_info *r2t,
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index b50aabe..715fefc 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -118,7 +118,8 @@ struct iscsi_transport {
 	int (*xmit_task) (struct iscsi_task *task);
 	void (*cleanup_task) (struct iscsi_task *task);
 
-	int (*alloc_pdu) (struct iscsi_task *task, uint8_t opcode);
+	int (*alloc_pdu) (struct iscsi_task *task, uint8_t opcode,
+			  unsigned int offset, unsigned int count);
 	int (*xmit_pdu) (struct iscsi_task *task);
 	int (*init_pdu) (struct iscsi_task *task, unsigned int offset,
 			 unsigned int count);

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu()
       [not found] ` <200902110301.n1B31q5M002101-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2009-02-11  8:21   ` Or Gerlitz
  0 siblings, 0 replies; 3+ messages in thread
From: Or Gerlitz @ 2009-02-11  8:21 UTC (permalink / raw)
  To: kxie-ut6Up61K2wZBDgjK7y7TUQ
  Cc: open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	michaelc-hcNo3dDEHLuVc3sceRu5cw,
	James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk


Karen Xie wrote:
> So I only added the offset and count as additional parameters to alloc_pdu(). 
> So that the pdu payload is known at the time of pdu memory allocation.
>  include/scsi/libiscsi.h             |   12 +++++
>  include/scsi/scsi_transport_iscsi.h |    3 +

Hi Karen,

Please take into account that the iscsi transport and libiscsi APIs has more consumers that are effects by changes. For example iSER also uses the alloc_pdu API and your patch should take care of that. I would recommend adding the needed CONFIG directives the other iSCSI transports to your default .config profile which will allow you to catch such issues. Building Linus tree with your patch I see warning both in the cxgbi and iser drivers wrt to the change.

Or.

[root@linux-cto-1 linus-linux-2.6]# make
  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  SYMLINK include/asm -> include/asm-x86
  CALL    scripts/checksyscalls.sh
  CHK     include/linux/compile.h
  CC [M]  drivers/infiniband/ulp/iser/iscsi_iser.o
drivers/infiniband/ulp/iser/iscsi_iser.c:656: warning: initialization from incompatible pointer type
  LD [M]  drivers/infiniband/ulp/iser/ib_iser.o
  CC [M]  drivers/scsi/cxgb3i/cxgb3i_iscsi.o
drivers/scsi/cxgb3i/cxgb3i_iscsi.c:913: warning: initialization from incompatible pointer type
  LD [M]  drivers/scsi/cxgb3i/cxgb3i.o
Kernel: arch/x86/boot/bzImage is ready  (#3)
  Building modules, stage 2.
  MODPOST 514 modules
  LD [M]  drivers/infiniband/ulp/iser/ib_iser.ko
  LD [M]  drivers/scsi/cxgb3i/cxgb3i.ko

the config directives for iser and ql4xxx are CONFIG_SCSI_QLA_ISCSI=m and CONFIG_INFINIBAND_ISER=m
for iser you would also need CONFIG_INFINIBAND=m and CONFIG_INFINIBAND_ADDR_TRANS=y

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu()
  2009-02-11  3:01 [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu() Karen Xie
       [not found] ` <200902110301.n1B31q5M002101-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2009-02-11 19:57 ` Mike Christie
  1 sibling, 0 replies; 3+ messages in thread
From: Mike Christie @ 2009-02-11 19:57 UTC (permalink / raw)
  To: open-iscsi; +Cc: linux-scsi, kxie, James.Bottomley

Karen Xie wrote:
> [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu().
> 
> From: Karen Xie <kxie@chelsio.com>
> 
> Hi, Mike,
> 
> I looked through libiscsi.c, libiscsi_tcp.c and iscsi_tcp.c. It does seem to be a little messy to merge the two functions. Especially the BHS is constructed after pdu_alloc(), and iscsi_tcp uses the BHS fields in init_pdu(). 
> 
> So I only added the offset and count as additional parameters to alloc_pdu(). So that the pdu payload is known at the time of pdu memory allocation.
> 

Ehhhhhh, yeah it is workable. Give me a day to check this out more. It 
makes the api akward because we get the data offset and len, but not the 
header. Let me see if I can build on your patch.

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-02-11 19:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-11  3:01 [PATCH 2.6.29-rc] iscsi - add offset and count to alloc_pdu() Karen Xie
     [not found] ` <200902110301.n1B31q5M002101-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-02-11  8:21   ` Or Gerlitz
2009-02-11 19:57 ` Mike Christie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.