Another bugfix for 2.6.26

Another bugfix for 2.6.26

[michaelc]

Sorry about multiple days of patches James. I found another possible bug
in the iscsi code. The problem occurs when setting the iscsi recv timeout
and iscsi ping timeout to signifigantly different values.

The patch was made over scsi-rc-fixes.

[PATCH 1/1] libiscsi regression in 2.6.25: fix setting of recv timer

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If the ping tmo is longer than the recv tmo then we could miss a window
where we were supposed to check the recv tmo. This happens because
the ping code will set the next timeout for the ping timeout, and if the
ping executes quickly there will be a long chunk of time before the
timer wakes up again.

This patch has the ping processing code kick off a recv
tmo check when getting a nop in response to our ping.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 98164f3..b43bf1d 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -730,7 +730,9 @@ static int __iscsi_complete_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
 				if (iscsi_recv_pdu(conn->cls_conn, hdr, data,
 						   datalen))
 					rc = ISCSI_ERR_CONN_FAILED;
-			}
+			} else
+				mod_timer(&conn->transport_timer,
+					  jiffies + conn->recv_timeout);
 			iscsi_free_mgmt_task(conn, mtask);
 			break;
 		default:
@@ -1478,11 +1480,9 @@ static void iscsi_check_transport_timeouts(unsigned long data)
 	}
 
 	if (time_before_eq(last_recv + recv_timeout, jiffies)) {
-		if (time_before_eq(conn->last_ping, last_recv)) {
-			/* send a ping to try to provoke some traffic */
-			debug_scsi("Sending nopout as ping on conn %p\n", conn);
-			iscsi_send_nopout(conn, NULL);
-		}
+		/* send a ping to try to provoke some traffic */
+		debug_scsi("Sending nopout as ping on conn %p\n", conn);
+		iscsi_send_nopout(conn, NULL);
 		next_timeout = conn->last_ping + (conn->ping_timeout * HZ);
 	} else
 		next_timeout = last_recv + recv_timeout;
-- 
1.5.4.1

Re: [PATCH 1/1] libiscsi regression in 2.6.25: fix setting of recv timer

[Bart Van Assche]

On Fri, May 9, 2008 at 3:15 AM,  <michaelc@cs.wisc.edu> wrote:
> From: Mike Christie <michaelc@cs.wisc.edu>
>
> If the ping tmo is longer than the recv tmo then we could miss a window
> where we were supposed to check the recv tmo. This happens because
> the ping code will set the next timeout for the ping timeout, and if the
> ping executes quickly there will be a long chunk of time before the
> timer wakes up again.
>
> This patch has the ping processing code kick off a recv
> tmo check when getting a nop in response to our ping.
>
> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

Hello Mike,

Did you already consider to submit this patch for inclusion in a
2.6.25.x stable release ? See also
Documentation/stable_kernel_rules.txt for the details.

Bart.

Re: [PATCH 1/1] libiscsi regression in 2.6.25: fix setting of recv timer

[James Bottomley]

On Fri, 2008-05-09 at 16:57 +0200, Bart Van Assche wrote:
> On Fri, May 9, 2008 at 3:15 AM,  <michaelc@cs.wisc.edu> wrote:
> > From: Mike Christie <michaelc@cs.wisc.edu>
> >
> > If the ping tmo is longer than the recv tmo then we could miss a window
> > where we were supposed to check the recv tmo. This happens because
> > the ping code will set the next timeout for the ping timeout, and if the
> > ping executes quickly there will be a long chunk of time before the
> > timer wakes up again.
> >
> > This patch has the ping processing code kick off a recv
> > tmo check when getting a nop in response to our ping.
> >
> > Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
> 
> Hello Mike,
> 
> Did you already consider to submit this patch for inclusion in a
> 2.6.25.x stable release ? See also
> Documentation/stable_kernel_rules.txt for the details.

He can't; if you actually read the rules, the last one says:

 - It or an equivalent fix must already exist in Linus' tree.  Quote the
   respective commit ID in Linus' tree in your patch submission to -stable.

This requires that it be upstream.

There's an automatic tag I do which sends the patch to the stable tree
when it goes upstream from my tree, but the author cannot himself
simultaneously submit it to me and to stable because it's not eligible
until it is upstream.

James

Re: [PATCH 1/1] libiscsi regression in 2.6.25: fix setting of recv timer

[Mike Christie]

Bart Van Assche wrote:
> On Fri, May 9, 2008 at 3:15 AM,  <michaelc@cs.wisc.edu> wrote:
>> From: Mike Christie <michaelc@cs.wisc.edu>
>>
>> If the ping tmo is longer than the recv tmo then we could miss a window
>> where we were supposed to check the recv tmo. This happens because
>> the ping code will set the next timeout for the ping timeout, and if the
>> ping executes quickly there will be a long chunk of time before the
>> timer wakes up again.
>>
>> This patch has the ping processing code kick off a recv
>> tmo check when getting a nop in response to our ping.
>>
>> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
> 
> Hello Mike,
> 
> Did you already consider to submit this patch for inclusion in a
> 2.6.25.x stable release ? See also
> Documentation/stable_kernel_rules.txt for the details.
> 

James does some magic where if we mark it as a regression against a 
previous kernel, when he merges it to send it upstream he will send cc 
the stable kernel guys for us. It is pretty neat like some sort of 
JamesBot :)