From mboxrd@z Thu Jan 1 00:00:00 1970 From: devik Date: Wed, 23 Jul 2003 07:39:55 +0000 Subject: Re: [LARTC] [HTB] htb_dequeue_tree assertion (kernel 2.4.21-ac4) MIME-Version: 1 Content-Type: multipart/mixed; boundary="8323328-446180073-1058945995=:18108" Message-Id: List-Id: References: In-Reply-To: To: lartc@vger.kernel.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. Send mail to mime@docserver.cac.washington.edu for more info. --8323328-446180073-1058945995=:18108 Content-Type: TEXT/PLAIN; charset=US-ASCII Hi, try attached fix please (it duplicates last one too so that you might get a reject). ------------------------------- Martin Devera aka devik Linux kernel QoS/HTB maintainer http://luxik.cdi.cz/~devik/ On Sun, 20 Jul 2003, Wilfried Weissmann 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. > >>>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 drop > >>packets or if you completely exchange the queue. Which would also > >>explain why the assertion only occurs when the configuration is altered. > > > > > > 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 queue. > > 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 every > 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. > > bye, > wilfried > > > > > devik > > > > > > ------------------------------------------------------------------------ > > > > --- sch_htb.c 2003/07/05 10:37:11 1.21 > > +++ sch_htb.c 2003/07/20 07:24:59 > > @@ -1286,6 +1286,10 @@ static int htb_graft(struct Qdisc *sch, > > return -ENOBUFS; > > sch_tree_lock(sch); > > if ((*old = xchg(&cl->un.leaf.q, new)) != NULL) { > > + /* TODO: test it */ > > + if (cl->prio_activity) > > + htb_deactivate ((struct htb_sched*)sch->data,cl); > > + > > /* TODO: is it correct ? Why CBQ doesn't do it ? */ > > sch->q.qlen -= (*old)->q.qlen; > > qdisc_reset(*old); > > > > > > --8323328-446180073-1058945995=:18108 Content-Type: TEXT/PLAIN; charset=US-ASCII; name="htb_deq_tree_fix1.diff" Content-Transfer-Encoding: BASE64 Content-ID: Content-Description: Content-Disposition: attachment; filename="htb_deq_tree_fix1.diff" LS0tIHNjaF9odGIuYwkyMDAzLzA3LzA1IDEwOjM3OjExCTEuMjENCisrKyBz Y2hfaHRiLmMJMjAwMy8wNy8yMyAwNzozNzo1Mg0KQEAgLTk0NywxNSArOTQ3 LDI0IEBAIHN0YXRpYyBzdHJ1Y3Qgc2tfYnVmZiAqDQogaHRiX2RlcXVldWVf dHJlZShzdHJ1Y3QgaHRiX3NjaGVkICpxLGludCBwcmlvLGludCBsZXZlbCkN CiB7DQogCXN0cnVjdCBza19idWZmICpza2IgPSBOVUxMOw0KLQkvL3N0cnVj dCBodGJfc2NoZWQgKnEgPSAoc3RydWN0IGh0Yl9zY2hlZCAqKXNjaC0+ZGF0 YTsNCiAJc3RydWN0IGh0Yl9jbGFzcyAqY2wsKnN0YXJ0Ow0KIAkvKiBsb29r IGluaXRpYWwgY2xhc3MgdXAgaW4gdGhlIHJvdyAqLw0KIAlzdGFydCA9IGNs ID0gaHRiX2xvb2t1cF9sZWFmIChxLT5yb3dbbGV2ZWxdK3ByaW8scHJpbyxx LT5wdHJbbGV2ZWxdK3ByaW8pOw0KIAkNCiAJZG8gew0KLQkJQlVHX1RSQVAo Y2wgJiYgY2wtPnVuLmxlYWYucS0+cS5xbGVuKTsgaWYgKCFjbCkgcmV0dXJu IE5VTEw7DQorCQlCVUdfVFJBUChjbCk7IA0KKwkJaWYgKCFjbCkgcmV0dXJu IE5VTEw7DQogCQlIVEJfREJHKDQsMSwiaHRiX2RlcV90ciBwcmlvPSVkIGxl dj0lZCBjbD0lWCBkZWZpYz0lZFxuIiwNCiAJCQkJcHJpbyxsZXZlbCxjbC0+ Y2xhc3NpZCxjbC0+dW4ubGVhZi5kZWZpY2l0W2xldmVsXSk7DQorDQorCQkv KiBjbGFzcyBjYW4gYmUgZW1wdHkgLSBpdCBpcyB1bmxpa2VseSBidXQgY2Fu IGJlIHRydWUgaWYgbGVhZg0KKwkJICAgcWRpc2MgZHJvcHMgcGFja2V0cyBp biBlbnF1ZXVlIHJvdXRpbmUgb3IgaWYgc29tZW9uZSB1c2VkDQorCQkgICBn cmFmdCBvcGVyYXRpb24gb24gdGhlIGxlYWYgc2luY2UgbGFzdCBkZXF1ZXVl OyANCisJCSAgIHNpbXBseSBkZWFjdGl2YXRlIGFuZCBza2lwIHN1Y2ggY2xh c3MgKi8NCisJCWlmICh1bmxpa2VseShjbC0+dW4ubGVhZi5xLT5xLnFsZW4g PT0gMCkpIHsNCisJCQlodGJfZGVhY3RpdmF0ZShxLGNsKTsNCisJCQlnb3Rv IG5ld19sb29rdXA7DQorCQl9DQogCQ0KIAkJaWYgKGxpa2VseSgoc2tiID0g Y2wtPnVuLmxlYWYucS0+ZGVxdWV1ZShjbC0+dW4ubGVhZi5xKSkgIT0gTlVM TCkpIA0KIAkJCWJyZWFrOw0KQEAgLTk2NSw2ICs5NzQsNyBAQCBodGJfZGVx dWV1ZV90cmVlKHN0cnVjdCBodGJfc2NoZWQgKnEsaW50DQogCQl9DQogCQlx LT5ud2NfaGl0Kys7DQogCQlodGJfbmV4dF9yYl9ub2RlKChsZXZlbD9jbC0+ cGFyZW50LT51bi5pbm5lci5wdHI6cS0+cHRyWzBdKStwcmlvKTsNCituZXdf bG9va3VwOg0KIAkJY2wgPSBodGJfbG9va3VwX2xlYWYgKHEtPnJvd1tsZXZl bF0rcHJpbyxwcmlvLHEtPnB0cltsZXZlbF0rcHJpbyk7DQogCX0gd2hpbGUg KGNsICE9IHN0YXJ0KTsNCiANCkBAIC0xMjg2LDYgKzEyOTYsMTAgQEAgc3Rh dGljIGludCBodGJfZ3JhZnQoc3RydWN0IFFkaXNjICpzY2gsIA0KIAkJCQkJ cmV0dXJuIC1FTk9CVUZTOw0KIAkJc2NoX3RyZWVfbG9jayhzY2gpOw0KIAkJ aWYgKCgqb2xkID0geGNoZygmY2wtPnVuLmxlYWYucSwgbmV3KSkgIT0gTlVM TCkgew0KKwkJCS8qIFRPRE86IHRlc3QgaXQgKi8NCisJCQlpZiAoY2wtPnBy aW9fYWN0aXZpdHkpDQorCQkJCWh0Yl9kZWFjdGl2YXRlICgoc3RydWN0IGh0 Yl9zY2hlZCopc2NoLT5kYXRhLGNsKTsNCisNCiAJCQkvKiBUT0RPOiBpcyBp dCBjb3JyZWN0ID8gV2h5IENCUSBkb2Vzbid0IGRvIGl0ID8gKi8NCiAJCQlz Y2gtPnEucWxlbiAtPSAoKm9sZCktPnEucWxlbjsJDQogCQkJcWRpc2NfcmVz ZXQoKm9sZCk7DQo= --8323328-446180073-1058945995=:18108-- _______________________________________________ LARTC mailing list / LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/mailman/listinfo/lartc HOWTO: http://lartc.org/