* [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
@ 2004-06-29 21:00 Stephen Hemminger
2004-06-29 21:45 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2004-06-29 21:00 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
This code in the csz scheduler, is just plain broken. The TDIFF_SAFE
effectively expands to:
unsigned long delay = now - q->t_c;
if (delay > 0) {
delay = 0;
goto do_reset;
}
if (delay >> q->delta_log)
So delay is always 0! I assume that what was originally intended
is the to keep delay bounded to 1<<q->delta_log.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
diff -Nru a/net/sched/sch_csz.c b/net/sched/sch_csz.c
--- a/net/sched/sch_csz.c 2004-06-29 11:32:40 -07:00
+++ b/net/sched/sch_csz.c 2004-06-29 11:32:40 -07:00
@@ -378,10 +378,8 @@
unsigned long R_c;
PSCHED_GET_TIME(now);
- delay = PSCHED_TDIFF_SAFE(now, q->t_c, 0, goto do_reset);
-
+ delay = PSCHED_TDIFF(now, q->t_c);
if (delay>>q->delta_log) {
-do_reset:
/* Delta is too large.
It is possible if MTU/BW > 1<<q->delta_log
(i.e. configuration error) or because of hardware
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
2004-06-29 21:00 [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz Stephen Hemminger
@ 2004-06-29 21:45 ` David S. Miller
2004-06-29 22:22 ` jamal
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2004-06-29 21:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Tue, 29 Jun 2004 14:00:16 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:
> This code in the csz scheduler, is just plain broken. The TDIFF_SAFE
> effectively expands to:
> unsigned long delay = now - q->t_c;
> if (delay > 0) {
> delay = 0;
> goto do_reset;
> }
> if (delay >> q->delta_log)
>
> So delay is always 0! I assume that what was originally intended
> is the to keep delay bounded to 1<<q->delta_log.
This bug has been there since day one, wow.
Good spotting, applied.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
2004-06-29 21:45 ` David S. Miller
@ 2004-06-29 22:22 ` jamal
2004-06-29 22:28 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: jamal @ 2004-06-29 22:22 UTC (permalink / raw)
To: David S. Miller; +Cc: Stephen Hemminger, netdev
That scheduler is only a relic.
Nobody uses it ;-> Check the cofig help.
Pretty sure if you study it youll find a lot more bugs.
cheers,
jamal
On Tue, 2004-06-29 at 17:45, David S. Miller wrote:
> On Tue, 29 Jun 2004 14:00:16 -0700
> Stephen Hemminger <shemminger@osdl.org> wrote:
>
> > This code in the csz scheduler, is just plain broken. The TDIFF_SAFE
> > effectively expands to:
> > unsigned long delay = now - q->t_c;
> > if (delay > 0) {
> > delay = 0;
> > goto do_reset;
> > }
> > if (delay >> q->delta_log)
> >
> > So delay is always 0! I assume that what was originally intended
> > is the to keep delay bounded to 1<<q->delta_log.
>
> This bug has been there since day one, wow.
>
> Good spotting, applied.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
2004-06-29 22:22 ` jamal
@ 2004-06-29 22:28 ` David S. Miller
2004-06-29 22:43 ` jamal
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2004-06-29 22:28 UTC (permalink / raw)
To: hadi; +Cc: shemminger, netdev
On 29 Jun 2004 18:22:37 -0400
jamal <hadi@cyberus.ca> wrote:
> That scheduler is only a relic.
> Nobody uses it ;-> Check the cofig help.
> Pretty sure if you study it youll find a lot more bugs.
Right, but it is good that we fix the obvious ones when
we can. Stephen's fix resulted in being able to simplify
some PSCHED_* macros significantly.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
2004-06-29 22:28 ` David S. Miller
@ 2004-06-29 22:43 ` jamal
2004-06-29 22:49 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: jamal @ 2004-06-29 22:43 UTC (permalink / raw)
To: David S. Miller; +Cc: shemminger, netdev
Sounds like good logic to me.
BTW, that jiffies fix, statemnt from Steve:
> The downside is the overhead of a function call, and a cache miss in >
>get_jiffies_64.
Is this overhead on 32 bit machines or applies on 64 bit as well?
Note that code is used in the fast path.
cheers,
jamal
On Tue, 2004-06-29 at 18:28, David S. Miller wrote:
> On 29 Jun 2004 18:22:37 -0400
> jamal <hadi@cyberus.ca> wrote:
>
> > That scheduler is only a relic.
> > Nobody uses it ;-> Check the cofig help.
> > Pretty sure if you study it youll find a lot more bugs.
>
> Right, but it is good that we fix the obvious ones when
> we can. Stephen's fix resulted in being able to simplify
> some PSCHED_* macros significantly.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz
2004-06-29 22:43 ` jamal
@ 2004-06-29 22:49 ` Stephen Hemminger
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2004-06-29 22:49 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
On 29 Jun 2004 18:43:45 -0400
jamal <hadi@cyberus.ca> wrote:
>
> Sounds like good logic to me.
> BTW, that jiffies fix, statemnt from Steve:
> > The downside is the overhead of a function call, and a cache miss in >
> >get_jiffies_64.
>
> Is this overhead on 32 bit machines or applies on 64 bit as well?
> Note that code is used in the fast path.
jiffies.h
#if (BITS_PER_LONG < 64)
u64 get_jiffies_64(void);
#else
static inline u64 get_jiffies_64(void)
{
return (u64)jiffies;
}
#endif
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-06-29 22:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-29 21:00 [PATCH] (2/4) packet scheduler bad TDIFF_SAFE in csz Stephen Hemminger
2004-06-29 21:45 ` David S. Miller
2004-06-29 22:22 ` jamal
2004-06-29 22:28 ` David S. Miller
2004-06-29 22:43 ` jamal
2004-06-29 22:49 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).