On 06/24/2009 09:40 AM, Hannes Reinecke wrote:
> During heavy I/O we might hit a receive timeout as the
> xmitworker is still busy sending PDUs. Even as we strictly
> speaking didn't receive a reply during the receive timeout,
> we didn't actually gave the target a chance to reply as
> we're constantly hitting it with requests.
> So it's better to ensure that cmdqueue really is empty
> before start sending NOPs.
>
> Signed-off-by: Hannes Reinecke<h...@suse.de>
> ---
>   drivers/scsi/libiscsi.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 716cc34..41bb177 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1862,7 +1862,9 @@ static void iscsi_check_transport_timeouts(unsigned 
> long data)
>               return;
>       }
>
> -     if (time_before_eq(last_recv + recv_timeout, jiffies)) {
> +     if (time_before_eq(last_recv + recv_timeout, jiffies)&&
> +         list_empty(&conn->cmdqueue)&&
> +         list_empty(&conn->requeue)) {
>               /* send a ping to try to provoke some traffic */
>               ISCSI_DBG_CONN(conn, "Sending nopout as ping\n");
>               iscsi_send_nopout(conn, NULL);

If the connection goes bad and the network layer stops sending IO (you 
hit that wait in sendmsg/sendpage) but there are tasks in the cmdqueue 
or requeue then how will we detect that the connection has gone bad 
(tasks will not be removed from the queues at this time)?

With this patch will we have to wait until the scsi eh fires and the 
tmfs also timeout and then the target reset code eventually gives up and 
drops the session.

I think we want to change
         if (time_before_eq(last_recv + recv_timeout, jiffies)) {
                 /* send a ping to try to provoke some traffic */
                 ISCSI_DBG_CONN(conn, "Sending nopout as ping\n");
                 iscsi_send_nopout(conn, NULL);
                 next_timeout = conn->last_ping + (conn->ping_timeout * HZ);
         } else
                 next_timeout = last_recv + recv_timeout;


to account for xmits and recvs like is done with tasks, so if we have 
been successfully sending or receiving data then do not send a ping.

Here is a patch. It is only compile tested.  I think we need locking 
around the last_xfer accesses or maybe make it an atomic or something 
(is there a atomic that matches the size of jiffies on all archs that is 
not expensive).

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@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
-~----------~----~----~----~------~----~------~--~---

diff --git a/drivers/scsi/cxgb3i/cxgb3i_pdu.c b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
index 7091050..0ccbc4f 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_pdu.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
@@ -388,14 +388,15 @@ int cxgb3i_conn_xmit_pdu(struct iscsi_task *task)
        if (err > 0) {
                int pdulen = err;
 
-       cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
-                       task, skb, skb->len, skb->data_len, err);
+               cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
+                               task, skb, skb->len, skb->data_len, err);
 
                if (task->conn->hdrdgst_en)
                        pdulen += ISCSI_DIGEST_SIZE;
                if (datalen && task->conn->datadgst_en)
                        pdulen += ISCSI_DIGEST_SIZE;
 
+               tcp_conn->iscsi_conn->last_xfer = jiffies;
                task->conn->txdata_octets += pdulen;
                return 0;
        }
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 518dbd9..9253617 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -255,6 +255,7 @@ static int iscsi_sw_tcp_xmit_segment(struct iscsi_tcp_conn 
*tcp_conn,
                        iscsi_tcp_segment_unmap(segment);
                        return r;
                }
+               tcp_conn->iscsi_conn->last_xfer = jiffies;
                copied += r;
        }
        return copied;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 716cc34..c714bc5 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -928,7 +928,7 @@ int __iscsi_complete_pdu(struct iscsi_conn *conn, struct 
iscsi_hdr *hdr,
        struct iscsi_task *task;
        uint32_t itt;
 
-       conn->last_recv = jiffies;
+       conn->last_xfer = jiffies;
        rc = iscsi_verify_itt(conn, hdr->itt);
        if (rc)
                return rc;
@@ -1738,7 +1738,7 @@ static void iscsi_start_tx(struct iscsi_conn *conn)
 static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
 {
        if (conn->ping_task &&
-           time_before_eq(conn->last_recv + (conn->recv_timeout * HZ) +
+           time_before_eq(conn->last_xfer + (conn->recv_timeout * HZ) +
                           (conn->ping_timeout * HZ), jiffies))
                return 1;
        else
@@ -1838,7 +1838,7 @@ static void iscsi_check_transport_timeouts(unsigned long 
data)
 {
        struct iscsi_conn *conn = (struct iscsi_conn *)data;
        struct iscsi_session *session = conn->session;
-       unsigned long recv_timeout, next_timeout = 0, last_recv;
+       unsigned long recv_timeout, next_timeout = 0, last_xfer;
 
        spin_lock(&session->lock);
        if (session->state != ISCSI_STATE_LOGGED_IN)
@@ -1849,26 +1849,26 @@ static void iscsi_check_transport_timeouts(unsigned 
long data)
                goto done;
 
        recv_timeout *= HZ;
-       last_recv = conn->last_recv;
+       last_xfer = conn->last_xfer;
 
        if (iscsi_has_ping_timed_out(conn)) {
                iscsi_conn_printk(KERN_ERR, conn, "ping timeout of %d secs "
                                  "expired, recv timeout %d, last rx %lu, "
                                  "last ping %lu, now %lu\n",
                                  conn->ping_timeout, conn->recv_timeout,
-                                 last_recv, conn->last_ping, jiffies);
+                                 last_xfer, conn->last_ping, jiffies);
                spin_unlock(&session->lock);
                iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
                return;
        }
 
-       if (time_before_eq(last_recv + recv_timeout, jiffies)) {
+       if (time_before_eq(last_xfer + recv_timeout, jiffies)) {
                /* send a ping to try to provoke some traffic */
                ISCSI_DBG_CONN(conn, "Sending nopout as ping\n");
                iscsi_send_nopout(conn, NULL);
                next_timeout = conn->last_ping + (conn->ping_timeout * HZ);
        } else
-               next_timeout = last_recv + recv_timeout;
+               next_timeout = last_xfer + recv_timeout;
 
        ISCSI_DBG_CONN(conn, "Setting next tmo %lu\n", next_timeout);
        mod_timer(&conn->transport_timer, next_timeout);
@@ -2611,7 +2611,7 @@ int iscsi_conn_start(struct iscsi_cls_conn *cls_conn)
        session->state = ISCSI_STATE_LOGGED_IN;
        session->queued_cmdsn = session->cmdsn;
 
-       conn->last_recv = jiffies;
+       conn->last_xfer = jiffies;
        conn->last_ping = jiffies;
        if (conn->recv_timeout && conn->ping_timeout)
                mod_timer(&conn->transport_timer,
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 2e0746d..5d2577a 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -870,7 +870,7 @@ int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct 
sk_buff *skb,
         * data_in's data could take a while to read in. We also want to
         * account for r2ts.
         */
-       conn->last_recv = jiffies;
+       conn->last_xfer = jiffies;
 
        if (unlikely(conn->suspend_rx)) {
                ISCSI_DBG_TCP(conn, "Rx suspended!\n");
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 61afeb5..5f90bcf 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -163,7 +163,7 @@ struct iscsi_conn {
         */
         int                    stop_stage;
        struct timer_list       transport_timer;
-       unsigned long           last_recv;
+       unsigned long           last_xfer;
        unsigned long           last_ping;
        int                     ping_timeout;
        int                     recv_timeout;

Reply via email to