Hi Dave,
Sacktag fastpath_cnt_hint seems to be very tricky to get right...

I suppose this one fixes Cedric's case. I cannot say for sure

until there is something more definite indication of

tcp_retrans_try_collapse origin than what the simple late WARN_ON

gave for us. ...Especially since it's non-trivial to have skb

hint "correctly" positioned in the write_queue while still ending

up calling that function. However, considering how difficult it

seems to be for Cedric to reproduce, it might well be this one.
In addition, I noticed another reset which wasn't previously

converted to WARN_ON, so doing that now. Boot + simple xfer

tested. Please apply to net-2.6.24.
--

 i.
-

Hello Ilpo !
I'm dropping the previous patches you sent me and switching to this patchset.

right ?
Thanks,
C.

-

On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo J

I have a spare node so I'm starting 2) with the 3 patches you sent and that
thanks to git ! :) 
C.
-

On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo J

I have 2 spare nodes so i'll run both. 1) is on already without any issues

i'm just compiling 2)
I usually work on -mm, so what would be interesting for me is to have what you

need in net-2.6.24 which is getting pulled in -mm by andrew. then, if you need

an extra patch for verbosity, that's fine, i'll include it in my usual patchset.
Cheers,
-

On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo J

Below are the messages I got on 2) right after running ketchup (which does

a wget www.kernel.org) 
He, you might have solved it with 1). If not, I'm keeping the hardware for

you.
Cheers,
C.
WARNING: at /home/legoater/linux/2.6.23-rc8-mm2-tcp_fastretrans/net/ipv4/tcp_ipv4.c:193 tcp_verify_fackets()
Call Trace:

 <IRQ>  [<ffffffff8041aa86>] tcp_verify_fackets+0x119/0x237

 [<ffffffff80416e57>] tcp_fragment+0x468/0x4b8

 [<ffffffff804184a5>] tcp_retransmit_skb+0xcf/0x2f4

 [<ffffffff8041878d>] tcp_xmit_retransmit_queue+0xc3/0x31e

 [<ffffffff8041220a>] tcp_fastretrans_alert+0xb36/0xb43

 [<ffffffff80412f0f>] tcp_ack+0x5d3/0x71b

 [<ffffffff80415229>] tcp_rcv_established+0x61f/0x6df

 [<ffffffff8025419a>] __lock_acquire+0x8a1/0xf1b

 [<ffffffff8041c7ff>] tcp_v4_do_rcv+0x3e/0x394

 [<ffffffff8041d171>] tcp_v4_rcv+0x61c/0x9a9

 [<ffffffff804017e3>] ip_local_deliver+0x1da/0x2a4

 [<ffffffff8040214e>] ip_rcv+0x583/0x5c9

 [<ffffffff8046fe43>] packet_rcv_spkt+0x19a/0x1a8

 [<ffffffff803e2e1c>] netif_receive_skb+0x2cf/0x2f5

 [<ffffffff88042505>] :tg3:tg3_poll+0x65d/0x8a4

 [<ffffffff803e2fe8>] net_rx_action+0xb8/0x191

 [<ffffffff8023a9b7>] __do_softirq+0x5f/0xe0

 [<ffffffff8020c98c>] call_softirq+0x1c/0x28

 [<ffffffff8020e9c3>] do_softirq+0x3b/0xb8

 [<ffffffff8023aaae>] irq_exit+0x4e/0x50

 [<ffffffff8020e7df>] do_IRQ+0xbd/0xd7

 [<ffffffff80209cb9>] mwait_idle+0x0/0x4d

 [<ffffffff8020bce6>] ret_from_intr+0x0/0xf

 <EOI>  [<ffffffff80209cfc>] mwait_idle+0x43/0x4d

 [<ffffffff802099fb>] enter_idle+0x22/0x24

 [<ffffffff80209c4f>] cpu_idle+0x9d/0xc0

 [<ffffffff80479591>] rest_init+0x55/0x57

 [<ffffffff8063681f>] start_kernel+0x2e0/0x2ec

 [<ffffffff80636134>] _sinittext+0x134/0x13b
WARNING: at /home/legoater/linux/2.6.23-rc8-mm2-tcp_fastretrans/net/ipv4/tcp_ipv4.c:198 tcp_verify_fackets()
Call Trace:

 &l...
[ message continues ]

bummer, I got this one on 1) :(
C.
WARNING: at /home/legoater/linux/net-2.6.24.git/net/ipv4/tcp_input.c:2325 tcp_fastretrans_alert()
Call Trace:

 <IRQ>  [<ffffffff8022ddb6>] __wake_up+0x1f/0x4c

 [<ffffffff803fd9d3>] tcp_ack+0xcee/0x18ac

 [<ffffffff80400764>] tcp_rcv_established+0x61f/0x6df

 [<ffffffff8024e8d8>] __lock_acquire+0x8a1/0xf1b

 [<ffffffff8040795b>] tcp_v4_do_rcv+0x3e/0x394

 [<ffffffff804082d5>] tcp_v4_rcv+0x624/0x9b1

 [<ffffffff803ecfa3>] ip_local_deliver+0x1da/0x2a4

 [<ffffffff803ed900>] ip_rcv+0x57c/0x5c4

 [<ffffffff8045ae53>] packet_rcv_spkt+0x19a/0x1a8

 [<ffffffff803ce78e>] netif_receive_skb+0x2ba/0x2de

 [<ffffffff88044505>] :tg3:tg3_poll+0x65d/0x8a4

 [<ffffffff803ce958>] net_rx_action+0xb8/0x191

 [<ffffffff802385cb>] __do_softirq+0x5f/0xe0

 [<ffffffff8020c97c>] call_softirq+0x1c/0x28

 [<ffffffff8020e672>] do_softirq+0x3b/0xb9

 [<ffffffff802386c2>] irq_exit+0x4e/0x50

 [<ffffffff8020e48e>] do_IRQ+0xbe/0xd8

 [<ffffffff80209cb9>] mwait_idle+0x0/0x4d

 [<ffffffff8020bcc6>] ret_from_intr+0x0/0xf

 <EOI>  [<ffffffff80464e10>] __sched_text_start+0x5f0/0x62b

 [<ffffffff80464e10>] __sched_text_start+0x5f0/0x62b

 [<ffffffff80209cfc>] mwait_idle+0x43/0x4d

 [<ffffffff802099fb>] enter_idle+0x22/0x24

 [<ffffffff80209c4f>] cpu_idle+0x9d/0xc0

 [<ffffffff80464513>] rest_init+0x57/0x59

 [<ffffffff8060c82a>] start_kernel+0x2d1/0x2dd

 [<ffffffff8060c14e>] _sinittext+0x14e/0x155
WARNING: at /home/legoater/linux/net-2.6.24.git/net/ipv4/tcp_input.c:2325 tcp_fastretrans_alert()
Call Trace:

 <IRQ>  [<ffffffff8022ddb6>] __wake_up+0x1f/0x4c

 [<ffffffff803fd9d3>] tcp_ack+0xcee/0x18ac

 [<ffffffff80400764>] tcp_rcv_established+0x61f/0x6df

 [<ffffffff8024e8d8>] __lock_acquire+0x8a1/0xf1b

 [<ffffffff8040795b>] tcp_v4_do_rcv+0x3e/0x394

 [<ffffffff804082d5>] tcp_v4_rcv+0x6...
[ message continues ]

Oops, those tcp_fragment WARNINGs in the other mail were due to bug in

the debug patch as it called verify too early in there (before queue was

adjusted, no wonder it finds state inconsistent at that point, fixed that)...
...So please discard all old debug patches, they're all broken in this 
...I just wonder why that's the first place where it occurs... Can you try

the debug patch below (fixed verify place in tcp_fragment/collapse, added

some of them to narrow it down, and handled GSO more user friendly way in

the printout). Put it on top of those three patches (mm should be fine :-)).

...I wish the verify triggers way before the fastretrans trap (for some

reason it didn't do that in the quoted trace, maybe I had some verifys

missing in that old patch or something)...
--

 i.
 include/net/tcp.h     |    3 +

 net/ipv4/tcp_input.c  |   41 +++++++++++++++---

 net/ipv4/tcp_ipv4.c   |  113 +++++++++++++++++++++++++++++++++++++++++++++++++

 net/ipv4/tcp_output.c |    4 +-

 4 files changed, 154 insertions(+), 7 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h

index 991ccdc..54a0d91 100644

--- a/include/net/tcp.h

+++ b/include/net/tcp.h

@@ -43,6 +43,9 @@
 #include <linux/seq_file.h>
+extern void tcp_verify_fackets(struct sock *sk);

+extern void tcp_print_queue(struct sock *sk);

+

 extern struct inet_hashinfo tcp_hashinfo;
 extern atomic_t tcp_orphan_count;

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c

index 87c9ef5..7707b1d 100644

--- a/net/ipv4/tcp_input.c

+++ b/net/ipv4/tcp_input.c

@@ -1140,7 +1140,7 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,

 	return dup_sack;

 }
-static int

+int

 tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)

 {

 	const struct inet_connection_sock *icsk = inet_csk(sk);

@@ -1159,6 +1159,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_

 	int i;

 	int first_sack_index;
+	if (WARN_ON(!tp...
[ message continues ]

so here are the results on a net-2.6.24 kernel. 
I've put the patchset here to make sure it's correct: 
	http://legoater.free.fr/patches/2.6.23/net-2.6.24.git-tcp_fastretrans/
and plenty of logs :
	http://legoater.free.fr/patches/2.6.23/net-2.6.24.git-tcp_fastretrans.me...
FYI, config is here :
	http://legoater.free.fr/patches/2.6.23/net-2.6.24.git-tcp_fastretrans.co...
C.

-

Thanks a lot, the bug itself seems to occur far more common than I

thought... It just very rarely manifests in significant behavior (requires

non-trivial ACK/SACK pattern to occur). This same bug is present in the

soon to be released 2.6.23 and in stable too though it's nearly impossible

to ever see that by human eye and there's a silent fackets_out "fix up" in

place due to known (and intentional) fackets_out inaccuracy (and nobody is

really looking that large number of TCP traces)...
The fix below, you can test it with the debug patch quite easily, all

those messages should of course vanish :-). In case you want to

test, please mix those three patches (fixes a real bug too), this one and

the debug patch together...
Dave should apply both the three patch series fix and this one as well to

net-2.6.24 (Dave, in case you want me to resubmit, just ask... :-)). I'll

send one against .23-rc9 soon after this (sadly enough, it will cause a

conflict...).
--

 i.
[PATCH net-2.6.24] [TCP]: Fix fastpath_cnt_hint when GSO skb is partially ACKed
I used to be under impression that all hints are cleared in

clean_rtx_queue whenever cumulative ACKs occur. Yet, when only

GSO skb was partially ACKed, no hints were reset, therefore

fastpath_cnt_hint must be tweaked too or else it can corrupt

fackets_out. In case there was also a skb that got fully ACKed,

the fastpath_skb_hint is set to NULL which causes a recount for

fastpath_cnt_hint (the old value won't be accessed anymore),

thus it can safely be decremented without additional checking.
Reported by Cedric Le Goater <clg@fr.ibm.com>, two other bugs

have been already found and patched due to that report, and

third one (unrelated to this fix) is under evaluation

currently :-)...
Signed-off-by: Ilpo J

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Ok, all the net-2.6.24 bits are in, I'll take care of -stable...

-

and a second run of ketchup gave the following.
cheers,
C.
WARNING: at /home/legoater/linux/2.6.23-rc8-mm2-tcp_fastretrans/net/ipv4/tcp_ipv4.c:193 tcp_verify_fackets()
Call Trace:

 <IRQ>  [<ffffffff8041aa86>] tcp_verify_fackets+0x119/0x237

 [<ffffffff80416e57>] tcp_fragment+0x468/0x4b8

 [<ffffffff804184a5>] tcp_retransmit_skb+0xcf/0x2f4

 [<ffffffff8041878d>] tcp_xmit_retransmit_queue+0xc3/0x31e

 [<ffffffff8041220a>] tcp_fastretrans_alert+0xb36/0xb43

 [<ffffffff80412f0f>] tcp_ack+0x5d3/0x71b

 [<ffffffff80415229>] tcp_rcv_established+0x61f/0x6df

 [<ffffffff8025419a>] __lock_acquire+0x8a1/0xf1b

 [<ffffffff8041c7ff>] tcp_v4_do_rcv+0x3e/0x394

 [<ffffffff8041d171>] tcp_v4_rcv+0x61c/0x9a9

 [<ffffffff804017e3>] ip_local_deliver+0x1da/0x2a4

 [<ffffffff8040214e>] ip_rcv+0x583/0x5c9

 [<ffffffff8046fe43>] packet_rcv_spkt+0x19a/0x1a8

 [<ffffffff803e2e1c>] netif_receive_skb+0x2cf/0x2f5

 [<ffffffff88042505>] :tg3:tg3_poll+0x65d/0x8a4

 [<ffffffff803e2fe8>] net_rx_action+0xb8/0x191

 [<ffffffff8023a9b7>] __do_softirq+0x5f/0xe0

 [<ffffffff8020c98c>] call_softirq+0x1c/0x28

 [<ffffffff8020e9c3>] do_softirq+0x3b/0xb8

 [<ffffffff8023aaae>] irq_exit+0x4e/0x50

 [<ffffffff8020e7df>] do_IRQ+0xbd/0xd7

 [<ffffffff80209cb9>] mwait_idle+0x0/0x4d

 [<ffffffff8020bce6>] ret_from_intr+0x0/0xf

 <EOI>  [<ffffffff80209cfc>] mwait_idle+0x43/0x4d

 [<ffffffff802099fb>] enter_idle+0x22/0x24

 [<ffffffff80209c4f>] cpu_idle+0x9d/0xc0

 [<ffffffff80479591>] rest_init+0x55/0x57

 [<ffffffff8063681f>] start_kernel+0x2e0/0x2ec

 [<ffffffff80636134>] _sinittext+0x134/0x13b
TCP wq(s) --S------<

TCP wq(i)   h      <

s1 f5 (14) p6 seq: su110259658 hs110265450 sn110278722 (0)

WARNING: at /home/legoater/linux/2.6.23-rc8-mm2-tcp_fastretrans/net/ipv4/tcp_ipv4.c:193 tcp_verify_fackets()
Call Trace:

 <IRQ>  [<ffffffff803250a...
[ message continues ]

1) Passing wrong skb to tcp_adjust_fackets_out could corrupt

fastpath_cnt_hint as tcp_skb_pcount(next_skb) is not included

to it if hint points exactly to the next_skb (it's lagging

behind, see sacktag).
2) When fastpath_skb_hint is put backwards to avoid dangling

skb reference, the skb's pcount must also be removed from count

(not included like above).
Reported by Cedric Le Goater <legoater@free.fr>
Signed-off-by: Ilpo J

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Applied, thanks Ilpo.

-

Signed-off-by: Ilpo J

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Applied.

-

This should no longer be necessary because fackets_out is

accurate. It indicates bugs elsewhere, thus report it.
Signed-off-by: Ilpo J

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Applied, thanks.

-