David Miller wrote: > From: Thomas Gleixner > Date: Thu, 09 Jul 2009 21:59:22 -0000 > >> The hrtimer callback cbq_undelay() is not serialized against >> cbq_ovl_delay(). That affects at least q->pmask and q->delay_timer. >> >> Lock it proper. >> >> Signed-off-by: Thomas Gleixner > > The problems here are even much deeper than it appears. > > First of all, I am to understand that hrtimers run from hardware > interrupt context, right? If so, all of these datastructures are > softirq safe only. > > And it is not merely the immediate things you see being modified in > this hrtimer, such as ->pmask etc., it is also the q->active[] > pointers, the list state for the classes, just about everything in the > qdisc state is referenced in this hrtimer code path. > > I wonder how many queer unexplainable bugs we see because of this. > > What should probably happen is that the hrtimer merely fires off work > at software interrupt context (perhaps a tasklet or similar), and that > software interrupt code take the qdisc's root lock throughout it's > execution. That's my understanding what HRTIMER_SOFTIRQ is used for. I think simply grabbing the root lock in cbq_undelay() should be fine. Compile-tested only.