[LARTC] [HTB] htb_dequeue_tree assertion (kernel 2.4.21-ac4)

[devik]

Yes I agree with you regarding zero queue size. I plan
to make patch similar to your proposal. I hope it will
be today.

-------------------------------
    Martin Devera aka devik
Linux kernel QoS/HTB maintainer
  http://luxik.cdi.cz/~devik/

On Mon, 21 Jul 2003 Wilfried.Weissmann@gmx.at wrote:

> > devik wrote:
> > >>>If you read comment above htb_dequeue_tree, it should be called
> > >>>only when it is sure that there are packets inside of the level/prio=
=2E
> > >>>It is known by other HTB mechanism (per-level activity lists).
> > >>>
> > >>>Thus the bugtrap is to catch case where class was inserted
> > >>>into activity list because it had packets in its sub-qdisc
> > >>>but when we actually decide to dequeue - it has no packet.
> > >>>It is weird - can qdisc lose packets even when dequeue was
> > >>>not called ??
> > >>
> > >>If you change the depth of the leave queue then it is possible to dro=
p
> > >>packets or if you completely exchange the queue. Which would also
> > >>explain why the assertion only occurs when the configuration is alter=
ed.
>
> Now I verified that the problem is indeed the 0 queue length and not a NU=
LL
> class pointer.
>
> > >
> > >
> > > Well, I agree that there is something wrong. Now it is neccessary to
> > > find scenario where it does happen so that it is fixed in right way.
> > > I have not much time these days to test these cases but your
> > informations
> > > would lead to following hypothesis:
> > >
> > > Classe's child qdisc is replaced while old one still has nonzero queu=
e.
> > > New empty qdisc is grafted under class instead. What about attached
> > > patch (it is against my latest version so you can see offset warnings=
) ?
> >
> > This would not work if there are several intermediates HTB queues from
> > the device to the leave queue. In this case every queue from the queue
> > that was changed to the root has to be notified about the change. (The
> > setup we want to use involves such a configuration.) Maybe it is better
> > to just deactivate a class when a dequeue from its leave failes due to =
a
> > zero queue length. If you are concerned about performance then an audit
> > process could be implemented. For example to check one leave queue ever=
y
> > 64 packets +/- initial random offset to create some entropy similar to
> > the maximum mount count in the ext2 filesystem. Maybe there are better
> > ways to do this. I am not so familiar with the code.
> >
> > I will make some tests with the patch tomorrow. If my theory is true
> > then it should still help a lot.
>
> With the patch applied it is much harder to find the right ceil settings =
to
> trigger the assertion, however it does not fix the problem. I also got th=
e
> following log entry:
>
> HTB: dequeue bug (8,270045,270045), report it please !
>
> Maybe this massages is just a side effect of the bug.
>
> Greetings,
> Wilfried
>
> >
> > bye,
> > wilfried
> >
> > >
> > > devik
> > >
> > >
> > > ---------------------------------------------------------------------=
---
> > >
> > > --- sch_htb.c=092003/07/05 10:37:11=091.21
> > > +++ sch_htb.c=092003/07/20 07:24:59
> > > @@ -1286,6 +1286,10 @@ static int htb_graft(struct Qdisc *sch,
> > >  =09=09=09=09=09return -ENOBUFS;
> > >  =09=09sch_tree_lock(sch);
> > >  =09=09if ((*old =3D xchg(&cl->un.leaf.q, new)) !=3D NULL) {
> > > +=09=09=09/* TODO: test it */
> > > +=09=09=09if (cl->prio_activity)
> > > +=09=09=09=09htb_deactivate ((struct htb_sched*)sch->data,cl);
> > > +
> > >  =09=09=09/* TODO: is it correct ? Why CBQ doesn't do it ? */
> > >  =09=09=09sch->q.qlen -=3D (*old)->q.qlen;
> > >  =09=09=09qdisc_reset(*old);
>
> --
> +++ GMX - Mail, Messaging & more  http://www.gmx.net +++
>
> Jetzt ein- oder umsteigen und USB-Speicheruhr als Pr=E4mie sichern!
>
>
>