[PATCH] remove interactive credit

[PATCH] remove interactive credit

[Con Kolivas]

[-- Attachment #1.1: Type: text/plain, Size: 28 bytes --]

remove interactive credit



[-- Attachment #1.2: sched-remove_interactive_credit.diff --]
[-- Type: text/x-patch, Size: 5032 bytes --]

Special casing tasks by interactive credit was helpful for preventing fully
cpu bound tasks from easily rising to interactive status. 

However it did not select out tasks that had periods of being fully cpu bound
and then sleeping while waiting on pipes, signals etc. This led to a more
disproportionate share of cpu time.

Backing this out will no longer special case only fully cpu bound tasks, and
prevents the variable behaviour that occurs at startup before tasks declare
themseleves interactive or not, and speeds up application startup slightly
under certain circumstances. It does cost in interactivity slightly as load
rises but it is worth it for the fairness gains.

Signed-off-by: Con Kolivas <kernel@kolivas.org>


Index: linux-2.6.10-rc1-mm2/include/linux/sched.h
===================================================================
--- linux-2.6.10-rc1-mm2.orig/include/linux/sched.h	2004-11-02 13:19:19.000000000 +1100
+++ linux-2.6.10-rc1-mm2/include/linux/sched.h	2004-11-02 13:20:03.000000000 +1100
@@ -527,7 +527,6 @@ struct task_struct {
 	prio_array_t *array;
 
 	unsigned long sleep_avg;
-	long interactive_credit;
 	unsigned long long timestamp, last_ran;
 	int activated;
 
Index: linux-2.6.10-rc1-mm2/kernel/sched.c
===================================================================
--- linux-2.6.10-rc1-mm2.orig/kernel/sched.c	2004-11-02 13:19:19.000000000 +1100
+++ linux-2.6.10-rc1-mm2/kernel/sched.c	2004-11-02 13:51:39.000000000 +1100
@@ -100,7 +100,6 @@
 #define MAX_SLEEP_AVG		(DEF_TIMESLICE * MAX_BONUS)
 #define STARVATION_LIMIT	(MAX_SLEEP_AVG)
 #define NS_MAX_SLEEP_AVG	(JIFFIES_TO_NS(MAX_SLEEP_AVG))
-#define CREDIT_LIMIT		100
 
 /*
  * If a task is 'interactive' then we reinsert it in the active
@@ -156,12 +155,6 @@
 	(JIFFIES_TO_NS(MAX_SLEEP_AVG * \
 		(MAX_BONUS / 2 + DELTA((p)) + 1) / MAX_BONUS - 1))
 
-#define HIGH_CREDIT(p) \
-	((p)->interactive_credit > CREDIT_LIMIT)
-
-#define LOW_CREDIT(p) \
-	((p)->interactive_credit < -CREDIT_LIMIT)
-
 #define TASK_PREEMPTS_CURR(p, rq) \
 	((p)->prio < (rq)->curr->prio)
 
@@ -669,8 +662,6 @@ static void recalc_task_prio(task_t *p, 
 			sleep_time > INTERACTIVE_SLEEP(p)) {
 				p->sleep_avg = JIFFIES_TO_NS(MAX_SLEEP_AVG -
 						DEF_TIMESLICE);
-				if (!HIGH_CREDIT(p))
-					p->interactive_credit++;
 		} else {
 			/*
 			 * The lower the sleep avg a task has the more
@@ -679,19 +670,18 @@ static void recalc_task_prio(task_t *p, 
 			sleep_time *= (MAX_BONUS - CURRENT_BONUS(p)) ? : 1;
 
 			/*
-			 * Tasks with low interactive_credit are limited to
-			 * one timeslice worth of sleep avg bonus.
+			 * Tasks are limited to one timeslice worth of
+			 * sleep avg bonus.
 			 */
-			if (LOW_CREDIT(p) &&
-			    sleep_time > JIFFIES_TO_NS(task_timeslice(p)))
+			if (sleep_time > JIFFIES_TO_NS(task_timeslice(p)))
 				sleep_time = JIFFIES_TO_NS(task_timeslice(p));
 
 			/*
-			 * Non high_credit tasks waking from uninterruptible
-			 * sleep are limited in their sleep_avg rise as they
-			 * are likely to be cpu hogs waiting on I/O
+			 * Tasks waking from uninterruptible sleep are 
+			 * limited in their sleep_avg rise as they
+			 * are likely to be waiting on I/O
 			 */
-			if (p->activated == -1 && !HIGH_CREDIT(p) && p->mm) {
+			if (p->activated == -1 && p->mm) {
 				if (p->sleep_avg >= INTERACTIVE_SLEEP(p))
 					sleep_time = 0;
 				else if (p->sleep_avg + sleep_time >=
@@ -711,11 +701,8 @@ static void recalc_task_prio(task_t *p, 
 			 */
 			p->sleep_avg += sleep_time;
 
-			if (p->sleep_avg > NS_MAX_SLEEP_AVG) {
+			if (p->sleep_avg > NS_MAX_SLEEP_AVG)
 				p->sleep_avg = NS_MAX_SLEEP_AVG;
-				if (!HIGH_CREDIT(p))
-					p->interactive_credit++;
-			}
 		}
 	}
 
@@ -1235,8 +1222,6 @@ void fastcall wake_up_new_task(task_t * 
 	p->sleep_avg = JIFFIES_TO_NS(CURRENT_BONUS(p) *
 		CHILD_PENALTY / 100 * MAX_SLEEP_AVG / MAX_BONUS);
 
-	p->interactive_credit = 0;
-
 	p->prio = effective_prio(p);
 
 	if (likely(cpu == this_cpu)) {
@@ -2702,12 +2687,10 @@ need_resched_nonpreemptible:
 		run_time = NS_MAX_SLEEP_AVG;
 
 	/*
-	 * Tasks with interactive credits get charged less run_time
-	 * at high sleep_avg to delay them losing their interactive
-	 * status
+	 * Tasks charged proportionately less run_time at high sleep_avg to
+	 * delay them losing their interactive status
 	 */
-	if (HIGH_CREDIT(prev))
-		run_time /= (CURRENT_BONUS(prev) ? : 1);
+	run_time /= (CURRENT_BONUS(prev) ? : 1);
 
 	spin_lock_irq(&rq->lock);
 
@@ -2795,11 +2778,8 @@ switch_tasks:
 	rcu_qsctr_inc(task_cpu(prev));
 
 	prev->sleep_avg -= run_time;
-	if ((long)prev->sleep_avg <= 0) {
+	if ((long)prev->sleep_avg <= 0)
 		prev->sleep_avg = 0;
-		if (!(HIGH_CREDIT(prev) || LOW_CREDIT(prev)))
-			prev->interactive_credit--;
-	}
 	prev->timestamp = prev->last_ran = now;
 
 	sched_info_switch(prev, next);
@@ -3899,7 +3879,6 @@ void __devinit init_idle(task_t *idle, i
 	unsigned long flags;
 
 	idle->sleep_avg = 0;
-	idle->interactive_credit = 0;
 	idle->array = NULL;
 	idle->prio = MAX_PRIO;
 	idle->state = TASK_RUNNING;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] remove interactive credit

[Nick Piggin]

Con Kolivas wrote:
> remove interactive credit
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Special casing tasks by interactive credit was helpful for preventing fully
> cpu bound tasks from easily rising to interactive status. 
> 
> However it did not select out tasks that had periods of being fully cpu bound
> and then sleeping while waiting on pipes, signals etc. This led to a more
> disproportionate share of cpu time.
> 
> Backing this out will no longer special case only fully cpu bound tasks, and
> prevents the variable behaviour that occurs at startup before tasks declare
> themseleves interactive or not, and speeds up application startup slightly
> under certain circumstances. It does cost in interactivity slightly as load
> rises but it is worth it for the fairness gains.
> 
> Signed-off-by: Con Kolivas <kernel@kolivas.org>
> 

I'm scared :(

I'm in favour of any attempts to simplify things... but will it be two
months or three before this spontaneously explodes for half our userbase?

Andrew's boss so he gets to decide >:)

Re: [PATCH] remove interactive credit

[Bill Davidsen]

Nick Piggin wrote:
> Con Kolivas wrote:
> 
>> remove interactive credit
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Special casing tasks by interactive credit was helpful for preventing 
>> fully
>> cpu bound tasks from easily rising to interactive status.
>> However it did not select out tasks that had periods of being fully 
>> cpu bound
>> and then sleeping while waiting on pipes, signals etc. This led to a more
>> disproportionate share of cpu time.
>>
>> Backing this out will no longer special case only fully cpu bound 
>> tasks, and
>> prevents the variable behaviour that occurs at startup before tasks 
>> declare
>> themseleves interactive or not, and speeds up application startup 
>> slightly
>> under certain circumstances. It does cost in interactivity slightly as 
>> load
>> rises but it is worth it for the fairness gains.
>>
>> Signed-off-by: Con Kolivas <kernel@kolivas.org>
>>
> 
> I'm scared :(
> 
> I'm in favour of any attempts to simplify things... but will it be two
> months or three before this spontaneously explodes for half our userbase?
> 
> Andrew's boss so he gets to decide >:)

If I read the intent right, this is an example of worst case avoidance, 
where a little average interactivity is traded for preventing the case 
where a single misguided process gets most/all of the CPU and 
performance falls off the edge of the table. As a user I see that as the 
same line of thought which gave us low-latency patches, to trade a 
miniscule bit of one thing to avoid something really undesirable.

I suspect there will be people who don't want to make this trade, 
although it sounds like a good one to me.

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

Re: [PATCH] remove interactive credit

[Ingo Molnar]

* Con Kolivas <kernel@kolivas.org> wrote:

> remove interactive credit

we could try this in -mm, but it obviously needs alot of testing first. 
Do you have any particular workload in mind where the fairness win due
to this revert would/should be significant?

	Ingo

Re: [PATCH] remove interactive credit

[Con Kolivas]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

Ingo Molnar wrote:
> * Con Kolivas <kernel@kolivas.org> wrote:
> 
> 
>>remove interactive credit
> 
> 
> we could try this in -mm, but it obviously needs alot of testing first. 
> Do you have any particular workload in mind where the fairness win due
> to this revert would/should be significant?

Since I created this variable in the first place I can say with quite 
some certainty that the size of the advantage is miniscule. Whereas 
clearly the design introduces special case mistreatment of only one type 
of task. It's an addition to the interactivity code I've often looked at 
and regretted doing.

Con

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] remove interactive credit

[Ingo Molnar]

* Con Kolivas <kernel@kolivas.org> wrote:

> >>remove interactive credit
> >
> >we could try this in -mm, but it obviously needs alot of testing first. 
> >Do you have any particular workload in mind where the fairness win due
> >to this revert would/should be significant?
> 
> Since I created this variable in the first place I can say with quite
> some certainty that the size of the advantage is miniscule. Whereas
> clearly the design introduces special case mistreatment of only one
> type of task. It's an addition to the interactivity code I've often
> looked at and regretted doing.

yeah, i know, it was the only piece of code from your earlier -Oint
scheduler-fixup series i almost didnt ack. But now it's in and testing
needs to cross at least one stable kernel boundary before it can be
taken out again. (unless a patch is an obvious or important fix.)

	Ingo

Re: [PATCH] remove interactive credit

[Con Kolivas]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Ingo Molnar wrote:
> * Con Kolivas <kernel@kolivas.org> wrote:
> 
> 
>>>>remove interactive credit
>>>
>>>we could try this in -mm, but it obviously needs alot of testing first. 
>>>Do you have any particular workload in mind where the fairness win due
>>>to this revert would/should be significant?
>>
>>Since I created this variable in the first place I can say with quite
>>some certainty that the size of the advantage is miniscule. Whereas
>>clearly the design introduces special case mistreatment of only one
>>type of task. It's an addition to the interactivity code I've often
>>looked at and regretted doing.
> 
> 
> yeah, i know, it was the only piece of code from your earlier -Oint
> scheduler-fixup series i almost didnt ack. But now it's in and testing
> needs to cross at least one stable kernel boundary before it can be
> taken out again. (unless a patch is an obvious or important fix.)

I've been extensively testing it at this end and recommend giving it a 
good -mm run. I'm reasonably sure it's related to some of the 
disproportionate cpu usage reports we've seen with some of those bash 
script examples (can't recall the details now so I'll need to chase them 
up). If I didn't suspect it was an issue I wouldn't be keen to undo 
something either.

Con

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] remove interactive credit

[Ingo Molnar]

* Con Kolivas <kernel@kolivas.org> wrote:

> >yeah, i know, it was the only piece of code from your earlier -Oint
> >scheduler-fixup series i almost didnt ack. But now it's in and testing
> >needs to cross at least one stable kernel boundary before it can be
> >taken out again. (unless a patch is an obvious or important fix.)
> 
> I've been extensively testing it at this end and recommend giving it a
> good -mm run. I'm reasonably sure it's related to some of the
> disproportionate cpu usage reports we've seen with some of those bash
> script examples (can't recall the details now so I'll need to chase
> them up). If I didn't suspect it was an issue I wouldn't be keen to
> undo something either.

i'm too quite positive about the expected effects. -mm testing will
tell.

	Ingo