On Tue, Dec 09, 2008 at 01:25:15PM +0100, Patrick McHardy wrote:

Jarek Poplawski wrote:

quoted

On Tue, Dec 09, 2008 at 11:28:44AM +0100, Patrick McHardy wrote:

quoted

quoted

-	/* too much load - let's continue on next jiffie (including above) */
-	return q->now + 2 * PSCHED_TICKS_PER_SEC / HZ;
+	/*
+	 * Too much load - let's continue on next jiffie.
+	 * (Used jiffies are charged later.)
+	 */
+	return q->now + 1;
 

This (including the last patch) is really confusing - q->now doesn't
contain HZ values but psched ticks. Could you describe the overall
algorithm you're trying to implement please?

The algorithm is we want to "continue on the next jiffie". We know
we've lost here a lot of time (~2 jiffies), and this will be added
later. Since these jiffies are not precise enough wrt. psched ticks
or ktime, and we will add around 2000 (for HZ 1000) psched ticks
anyway this +1 here simply doesn't matter and can mean "a bit after
q->now".

This might as well return q->now, no?

Yes, but IMHO it looks worse, considering the problem here (we want to
avoid scheduling in the past).

The elapsed time is added
on top later anyways. But anyways, I think both the approach and
the patch are wrong.

	/* charge used jiffies */
	start_at = jiffies - start_at;
	if (start_at > 0)
		next_event += start_at * PSCHED_TICKS_PER_SEC / HZ;

What relationship does the duration it ran for has to the time it
should run at again?

The scheduling times won't be in the past mostly and hrtimers won't
trigger too soon, but approximately around we really need and can
afford a new try without stopping everything else.

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.

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.

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.

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.
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.

Jarek P.