On Mon, Nov 06, 2006 at 09:44:49AM -0800, Stephen Hemminger wrote:
> On Mon, 6 Nov 2006 12:33:53 +0100
> Jarek Poplawski <[EMAIL PROTECTED]> wrote:
> 
> > After hlist_del() next and pprev pointers are not NULL
> > so hlist_unhashed() doesn't work properly.
> > 
> > 
> > Signed-off-by: Jarek Poplawski <[EMAIL PROTECTED]>
> > ---
> > 
> > 
> > diff -Nurp linux-2.6.19-rc4-git10-/net/sched/sch_htb.c 
> > linux-2.6.19-rc4-git10/net/sched/sch_htb.c
> > --- linux-2.6.19-rc4-git10-/net/sched/sch_htb.c     2006-11-06 
> > 11:42:41.000000000 +0100
> > +++ linux-2.6.19-rc4-git10/net/sched/sch_htb.c      2006-11-06 
> > 11:53:15.000000000 +0100
> > @@ -1284,8 +1284,10 @@ static void htb_destroy_class(struct Qdi
> >                                               struct htb_class, sibling));
> >  
> >     /* note: this delete may happen twice (see htb_delete) */
> > -   if (!hlist_unhashed(&cl->hlist))
> > +   if (!hlist_unhashed(&cl->hlist)) {
> >             hlist_del(&cl->hlist);
> > +           INIT_HLIST_NODE(&cl->hlist);
> > +   }
> 
> why not use hlist_del_init?

Yes, this is the question!

As a matter of fact I expected another question. Yesterday
I was short on time so I didn't describe the bug enough.
I'm not sure if you know the problem, so here are more
details (for me problem is 199% repeatable).

After something like this:

# tc qdisc add dev lo root handle 1: htb
# tc class add dev lo parent 1: classid 1:1 htb rate 200kbps
# tc class del dev lo classid 1:1

enter the BUG...

I've found the last command is the culprit and if you do:

# tc qdisc del dev lo root
there is no problem.

And probably it is enough to do the change only in htb_delete
- btw. is this hlist_del really needed there? and shouldn't
all deletions be done after zeroing the refcount? - but you
should know better. 

> 
> >     list_del(&cl->sibling);
> >  
> >     if (cl->prio_activity)
> > @@ -1333,8 +1335,10 @@ static int htb_delete(struct Qdisc *sch,
> >     sch_tree_lock(sch);
> >  
> >     /* delete from hash and active; remainder in destroy_class */
> > -   if (!hlist_unhashed(&cl->hlist))
> > +   if (!hlist_unhashed(&cl->hlist)) {
> >             hlist_del(&cl->hlist);
> > +           INIT_HLIST_NODE(&cl->hlist);
> > +   }
> >  
> >     if (cl->prio_activity)
> >             htb_deactivate(q, cl);

Best regards,

Jarek P.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to