Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs. dev_deactivate() race

[Jarek Poplawski <jarkao2@gmail.com>]

On Thu, Sep 11, 2008 at 09:49:29PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 11 Sep 2008 04:47:17 -0700 (PDT)
> 
> > Ok, so implementing ->peek() is the first step in dealing
> > with this.
> 
> Ok, here's a first cut at this.
> 
> Most of it is simple and straightforward.
> 
> As usual, though, CBQ, HFSC, and HTB are complicated.

I guess not for Patrick... (so CCed)

> 
> Most of the peek implementations just skb_peek() in their
> downstream queue or iterate over their prio array doing
> the same looking for a non-empty list.
> 
> But CBQ, HFSC, and HTB have complicated class iterators and
> internal time state machine things.  I tried to do my best
> in these cases.
> 
> I didn't want these things firing off class watchdog timers and
> stuff like this.  Just see if there is any packet ready now
> and return it.
> 
> The one exception is that I allow CBQ to advance it's time
> state machine.

Alas I'm still not sure how this whole peek idea is going to be
implemented (dequeuing after peeking doesn't have to give us the
same skb since in the meantime e.g. in HTB some other class with
higher prio can get enough tokens etc., and if there is a break
for transmit in the meantime with possible enqueuing, or we can
deal with something like sch_multiq, which depends on the current
state of the tx_queues, this all looks even more interesting).

So, until there is some example, even in pseudocode, for qdisc_run()
vs. some_classful_sched interaction, I think, I'm not able to give
more feedback now, but mabe only one doubt if this wrapper below is
really needed, since skb_peek() is more readable and it's used
directly in a few places anyway.

> +static inline struct sk_buff *__qdisc_peek_head(struct Qdisc *sch,
> +						struct sk_buff_head *list)
> +{
> +	return skb_peek(list);
> +}

Thanks,
Jarek P.