Subject: Re: [NET_SCHED 03/06]: Fix endless loops caused by inaccurate qlen counters (part 1)

From: Mika Penttilä <[email-masked]>
Date: Mon, 20 Nov 2006 18:07:51 +0200

Patrick McHardy wrote:
> Mika Penttilä wrote:
>   
>> Patrick McHardy wrote:
>>
>>     
>>> Mika Penttilä wrote:
>>>  
>>>
>>>       
>>>> Are you sure you didn't mean to :
>>>>
>>>> +    while ((parentid = sch->parent)) {
>>>> +        sch = __qdisc_lookup(sch->dev, TC_H_MAJ(parentid));
>>>> +        cops = sch->ops->cl_ops;
>>>> +        if (!(sch->q.qlen -= n) && cops->qlen_notify) {    <----
>>>>     
>>>>         
>>>
>>> We could do that, currently the qdiscs check themselves.
>>> The idea was mainly that they could be interested in
>>> other changes as well, for example for recalculating
>>> a deadline when the next packet in line changes.
>>>
>>>   
>>>       
>> But you call the notify before the decrement, which seems odd...
>>     
>
>
> The notification notifies of changes in a _child_ qdisc of
> the qdisc that is notified, which already has its counter
> decremented. The qdisc's own counter is irrelevant for
> the qdisc itself, it doesn't make any difference whether
> it is decremented before or after the notification.
>
> -
>   
Has already decremented where? As I read it you notify parent and 
_after_ that decrement child's counter...
Also, shoudn't the return value of qdisc_lookup be check, I think it can 
return NULL for default qdiscs.

--Mika