Jarek Poplawski wrote:

To David Miller: David don't apply yet - this patch needs change.

Patrick, read below:

On Tue, Dec 09, 2008 at 02:20:00PM +0100, Patrick McHardy wrote:

quoted

quoted

quoted

The focus on jiffies is wrong IMO, the reason why we get high
load is because the CPU can't keep up, delaying things even
longer is not going to help get the work done. The only reason to
look at jiffies is because its a cheap indication that it has
ran for too long and we should give other tasks a change to run
as well, but it should continue immediately after it did that.
So all it should do is make sure that the watchdog is scheduled
with a very small positive delay.

 >>>>

quoted

quoted

This needs additional psched_get_time(), and as I've written before
there is no apparent advantage in problematic cases, but this would
add more overhead for common cases.

 >>>

quoted

htb_do_events() exceeding two jiffies is fortunately not a common
case. You (incorrectly) made the calculation somewhat of a common
case by also adding to the delay if the inner classes simply throttled
and already returned the exact delay they want.

I see! You are right and this needs fixing.

quoted

Much better (again considering what we want to achieve here) would
be to not use the hrtimer watchdog at all. We want to give lower
priority tasks a chance to run, so ideally we'd use a low priority
task for wakeup.

I'm not sure I get ot right: for precise scheduling hrtimers look
useful. Do you really mean "at all"?

I meant "at all" for the wakeup after we've decided HTB has too much
work to do at once. A work queue seems better suited since that makes
sure we allow other processes to run, but don't wait unnecessarily
long when there is no other work.

quoted

quoted

quoted

As for the implementation: the increase in delay (the snippet
above) is also done in the case that no packets were available
for other reasons (throttling), in which case we might needlessly
delay for an extra jiffie if jiffies wrapped while it tried to
dequeue.

 >>>>

quoted

quoted

But in another similar cases there could be no change in jiffies, but
almost a jiffie used for counting, so wrong schedule time as well.

Its not "wrong". We don't want to delay. Its a courtesy to the
remaining system.

In this case it's probably self-courtesy too: this ksoftirqd takes
most of the time and it's useless.

Well, it calls back to HTB, which continues to do real work. But
leaving HTB, scheduling a timer just to be called immediately again
is useless overhead, I agree.

quoted

quoted

Approximatly this all should be fine, and it still can be tuned later.
IMHO, this all should not affect "common" cases, which are expected to
use less then jiffie here.

 >>>

quoted

Jiffies might wrap even if it only took only a few nanoseconds.
And its not fine, in the case of throttled classes there's no
reason to add extra delay *at all*.

Yes, you are right with this. I can try too fix this tomorrow, unless
you prefer to send your version of this patch.

I don't have a version of my own, so please go ahead :)