Mike Christie wrote:
> Doh forgot about the original issue. Nice idea and patch. Looks ok to me.

lets see... I was kind of under the impression that the --original-- or major 
issue here
is the session lock being held for two much time and introducing too much of a 
contention 
both between the command xmit / response flows and between multiple threads 
running the xmit
flow in passthrough mode. Today, I realized that on my current testbed, if I 
add the portion 
of the patch (below) which is buggy as it calls kfifo_put and updates the 
exp_cmdsn etc NOT under
the session lock - I get to a certain max in IOPS. I can't go over this max 
when applying my patch
of lock less flow for queuecommand / passthrough (see next mail). So with this 
observation at hand, I'd be happy if something can be done to allow for merging 
some version of the patch below before we touch the xmit path, if this possible 
at all...

[PATCH 10/9] ib/iser: remove locking from the scsi command response flow

optimize iser scsi command response processing to use libiscsi lockless 
completion path. This way there's no contention on the session lock 
between the scsi command submission to the scsi command completion flows.

Signed-off-by: Or Gerlitz <ogerl...@voltaire.com>

---

okay, this is buggy but buys me about more 50K IOPS! for four copies of 
disktest 
doing 1k reads with eight threads, each over a different session.

 drivers/infiniband/ulp/iser/iscsi_iser.c     |   29 ---------------------------
 drivers/infiniband/ulp/iser/iscsi_iser.h     |    5 ----
 drivers/infiniband/ulp/iser/iser_initiator.c |   20 +++++++++++++-----
 3 files changed, 15 insertions(+), 39 deletions(-)

Index: linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iser_initiator.c
===================================================================
--- linux-2.6.33-rc4.orig/drivers/infiniband/ulp/iser/iser_initiator.c
+++ linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -441,8 +441,9 @@ void iser_rcv_completion(struct iser_rx_
 {
        struct iscsi_iser_conn *conn = ib_conn->iser_conn;
        struct iscsi_hdr *hdr;
+       unsigned char opcode;
        u64 rx_dma;
-       int rx_buflen, outstanding, count, err;
+       int rx_buflen, outstanding, count, err, datalen;
 
        /* differentiate between login to all other PDUs */
        if ((char *)rx_desc == ib_conn->login_buf) {
@@ -458,11 +459,17 @@ void iser_rcv_completion(struct iser_rx_
 
        hdr = &rx_desc->iscsi_header;
 
-       iser_dbg("op 0x%x itt 0x%x dlen %d\n", hdr->opcode,
-                       hdr->itt, (int)(rx_xfer_len - ISER_HEADERS_LEN));
 
-       iscsi_iser_recv(conn->iscsi_conn, hdr,
-               rx_desc->data, rx_xfer_len - ISER_HEADERS_LEN);
+       opcode   = hdr->opcode & ISCSI_OPCODE_MASK;
+       datalen = (int)(rx_xfer_len - ISER_HEADERS_LEN);
+       iser_dbg("op 0x%x itt 0x%x dlen %d\n", opcode, hdr->itt, datalen);
+
+       if (opcode == ISCSI_OP_SCSI_CMD_RSP)
+               err = __iscsi_complete_pdu(conn->iscsi_conn, hdr,
+                                               rx_desc->data, datalen);
+       else
+               err = iscsi_complete_pdu(conn->iscsi_conn, hdr,
+                                               rx_desc->data, datalen);
 
        ib_dma_sync_single_for_device(ib_conn->device->ib_device, rx_dma,
                        rx_buflen, DMA_FROM_DEVICE);
@@ -473,6 +480,9 @@ void iser_rcv_completion(struct iser_rx_
         * for the posted rx bufs refcount to become zero handles everything   
*/
        conn->ib_conn->post_recv_buf_count--;
 
+       if (err)
+               iscsi_conn_failure(conn->iscsi_conn, err);
+
        if (rx_dma == ib_conn->login_dma)
                return;
 
Index: linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iscsi_iser.c
===================================================================
--- linux-2.6.33-rc4.orig/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -90,35 +90,6 @@ MODULE_PARM_DESC(debug_level, "Enable de
 
 struct iser_global ig;
 
-void
-iscsi_iser_recv(struct iscsi_conn *conn,
-               struct iscsi_hdr *hdr, char *rx_data, int rx_data_len)
-{
-       int rc = 0;
-       int datalen;
-       int ahslen;
-
-       /* verify PDU length */
-       datalen = ntoh24(hdr->dlength);
-       if (datalen != rx_data_len) {
-               printk(KERN_ERR "iscsi_iser: datalen %d (hdr) != %d (IB) \n",
-                      datalen, rx_data_len);
-               rc = ISCSI_ERR_DATALEN;
-               goto error;
-       }
-
-       /* read AHS */
-       ahslen = hdr->hlength * 4;
-
-       rc = iscsi_complete_pdu(conn, hdr, rx_data, rx_data_len);
-       if (rc && rc != ISCSI_ERR_NO_SCSI_CMD)
-               goto error;
-
-       return;
-error:
-       iscsi_conn_failure(conn, rc);
-}
-
 static int iscsi_iser_pdu_alloc(struct iscsi_task *task, uint8_t opcode)
 {
        struct iscsi_iser_task *iser_task = task->dd_data;
Index: linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iscsi_iser.h
===================================================================
--- linux-2.6.33-rc4.orig/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ linux-2.6.33-rc4/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -311,11 +311,6 @@ int iser_send_data_out(struct iscsi_conn
                       struct iscsi_task *task,
                       struct iscsi_data *hdr);
 
-void iscsi_iser_recv(struct iscsi_conn *conn,
-                    struct iscsi_hdr       *hdr,
-                    char                   *rx_data,
-                    int                    rx_data_len);
-
 void iser_conn_init(struct iser_conn *ib_conn);
 
 void iser_conn_get(struct iser_conn *ib_conn);

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-is...@googlegroups.com.
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

Reply via email to