Re: [PATCH 1/2] libiscsi: Do not sent NOP when xmit is in progress

2009-06-25 Thread Hannes Reinecke 

Mike Christie wrote:
 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 Reineckeh...@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)?
 
Hmm. Yes, indeed, this is a problem. But I think with your patch tracking
all xmits and recvs we don't need to apply that one as last_recv will
actually be last_xmit and hence a NOP will be sent if there is no traffic
at all.
Which as about what we want to achieve here.

 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).
 
No, we don't need a lock here as it's already been taken
(On the libiscsi side).

And this will probably also take care of the eh_timeout problem
as this will also check last_xmit.

I'll give it a spin.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

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

[PATCH 1/2] libiscsi: Do not sent NOP when xmit is in progress

2009-06-24 Thread Hannes Reinecke 


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);
-- 
1.5.3.2


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

Re: [PATCH 1/2] libiscsi: Do not sent NOP when xmit is in progress

2009-06-24 Thread Mike Christie 
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 Reineckeh...@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) +
+