On Tue, Aug 15, 2017 at 6:39 AM, Konstantin Khlebnikov <khlebni...@yandex-team.ru> wrote: > This callback is used for deactivating class in parent qdisc. > This is cheaper to test queue length right here. > > Also this allows to catch draining screwed backlog and prevent > second deactivation of already inactive parent class which will > crash kernel for sure. Kernel with print warning at destruction > of child qdisc where no packets but backlog is not zero.
Good cleanup. Please explicitly mark patches like this as for net-next. Just one comment below. > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c > index bd24a550e0f9..18da45c0769c 100644 > --- a/net/sched/sch_api.c > +++ b/net/sched/sch_api.c > @@ -752,6 +752,7 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, > unsigned int n, > const struct Qdisc_class_ops *cops; > unsigned long cl; > u32 parentid; > + bool notify; > int drops; > > if (n == 0 && len == 0) > @@ -764,6 +765,13 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, > unsigned int n, > > if (sch->flags & TCQ_F_NOPARENT) > break; > + /* Notify parent qdisc only if child qdisc becomes empty. > + * > + * If child was empty even before update then backlog > + * counter is screwed and we skip notification because > + * parent class is already passive. > + */ > + notify = !sch->q.qlen && !WARN_ON_ONCE(!n); Since 'n' never changes in this function, can we just move this WARN_ON_ONCE() right before the loop??
