| Message ID | 1325519277.2375.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Le lundi 02 janvier 2012 à 16:47 +0100, Eric Dumazet a écrit : > grp->slot_shift is between 22 and 41, so using 32bit wide variables is > probably a typo. > > This could explain QFQ hangs Dave reported to me, after 2^23 packets ? > > (23 = 64 - 41) > > Reported-by: Dave Taht <dave.taht@gmail.com> > CC: Stephen Hemminger <shemminger@vyatta.com> > CC: Dave Taht <dave.taht@gmail.com> Oh well, I forgot my SOB :( Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> David please just tell me if you prefer me to resend it -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 2, 2012 at 10:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le lundi 02 janvier 2012 à 16:47 +0100, Eric Dumazet a écrit : >> grp->slot_shift is between 22 and 41, so using 32bit wide variables is >> probably a typo. >> >> This could explain QFQ hangs Dave reported to me, after 2^23 packets ? >> >> (23 = 64 - 41) >> >> Reported-by: Dave Taht <dave.taht@gmail.com> >> CC: Stephen Hemminger <shemminger@vyatta.com> >> CC: Dave Taht <dave.taht@gmail.com> > > Oh well, I forgot my SOB :( > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > David please just tell me if you prefer me to resend it > > I figure I'm the wrong david, but I'll be testing the QFQ patch overnight. I have tested it for about an hour thus far. Your SFQ patch, posted earlier today, does bring SFQ and QFQ back into equivalence at 10 iperfs. http://www.teklibre.com/~d/bloat/sfqnewvsqfq10iperfs.png Well done.
On Mon, 02 Jan 2012 16:47:57 +0100 Eric Dumazet <eric.dumazet@gmail.com> wrote: > grp->slot_shift is between 22 and 41, so using 32bit wide variables is > probably a typo. > > This could explain QFQ hangs Dave reported to me, after 2^23 packets ? > > (23 = 64 - 41) > > Reported-by: Dave Taht <dave.taht@gmail.com> > CC: Stephen Hemminger <shemminger@vyatta.com> > CC: Dave Taht <dave.taht@gmail.com> > --- > net/sched/sch_qfq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > index 1033434..7b03254 100644 > --- a/net/sched/sch_qfq.c > +++ b/net/sched/sch_qfq.c > @@ -817,11 +817,11 @@ skip_unblock: > static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl) > { > unsigned long mask; > - uint32_t limit, roundedF; > + u64 limit, roundedF; > int slot_shift = cl->grp->slot_shift; > > roundedF = qfq_round_down(cl->F, slot_shift); > - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift); > + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift); > > if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) { > /* timestamp was stale */ > > You need to copy the BSD developers on these patches since most of the code came from them and FreeBSD probably still has same bug! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
thanks, appreciated. cheers luigi On Tue, Jan 03, 2012 at 08:25:57AM -0800, Stephen Hemminger wrote: > On Mon, 02 Jan 2012 16:47:57 +0100 > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > grp->slot_shift is between 22 and 41, so using 32bit wide variables is > > probably a typo. > > > > This could explain QFQ hangs Dave reported to me, after 2^23 packets ? > > > > (23 = 64 - 41) > > > > Reported-by: Dave Taht <dave.taht@gmail.com> > > CC: Stephen Hemminger <shemminger@vyatta.com> > > CC: Dave Taht <dave.taht@gmail.com> > > --- > > net/sched/sch_qfq.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > > index 1033434..7b03254 100644 > > --- a/net/sched/sch_qfq.c > > +++ b/net/sched/sch_qfq.c > > @@ -817,11 +817,11 @@ skip_unblock: > > static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl) > > { > > unsigned long mask; > > - uint32_t limit, roundedF; > > + u64 limit, roundedF; > > int slot_shift = cl->grp->slot_shift; > > > > roundedF = qfq_round_down(cl->F, slot_shift); > > - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift); > > + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift); > > > > if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) { > > /* timestamp was stale */ > > > > > > > You need to copy the BSD developers on these patches since > most of the code came from them and FreeBSD probably still has same bug! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 02 Jan 2012 16:47:57 +0100 > grp->slot_shift is between 22 and 41, so using 32bit wide variables is > probably a typo. > > This could explain QFQ hangs Dave reported to me, after 2^23 packets ? > > (23 = 64 - 41) > > Reported-by: Dave Taht <dave.taht@gmail.com> > CC: Stephen Hemminger <shemminger@vyatta.com> > CC: Dave Taht <dave.taht@gmail.com> Applied, thanks Eric. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c index 1033434..7b03254 100644 --- a/net/sched/sch_qfq.c +++ b/net/sched/sch_qfq.c @@ -817,11 +817,11 @@ skip_unblock: static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl) { unsigned long mask; - uint32_t limit, roundedF; + u64 limit, roundedF; int slot_shift = cl->grp->slot_shift; roundedF = qfq_round_down(cl->F, slot_shift); - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift); + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift); if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) { /* timestamp was stale */