[RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Suresh Jayaraman]

The newly introduced GENTLE_FAIR_SLEEPERS does not seem to have any
effect without FAIR_SLEEPERS. Fix sysctl.sched_features to reflect
this. Without this change, a user who sets GENTLE_FAIR_SLEEPERS
without FAIR_SLEEPERS would assume gentle sleeper fairness which
is not guaranteed.

Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---

Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -761,10 +761,22 @@ sched_feat_write(struct file *filp, cons
 		int len = strlen(sched_feat_names[i]);
 
 		if (strncmp(cmp, sched_feat_names[i], len) == 0) {
-			if (neg)
+			if (neg) {
 				sysctl_sched_features &= ~(1UL << i);
-			else
+				/*
+				 * GENTLE_FAIR_SLEEPERS have no effect without
+				 * FAIR_SLEEPERS.
+				 */
+				if (strncmp(cmp, "FAIR_SLEEPERS",
+					    strlen("FAIR_SLEEPERS")) == 0)
+					sysctl_sched_features &= ~(1UL << i+1);
+			} else {
 				sysctl_sched_features |= (1UL << i);
+				if (strncmp(cmp, "GENTLE_FAIR_SLEEPERS",
+					    strlen("GENTLE_FAIR_SLEEPERS"))
+						== 0)
+					sysctl_sched_features |= (1UL << i-1);
+			}
 			break;
 		}
 	}

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Ingo Molnar]

* Suresh Jayaraman <sjayaraman@suse.de> wrote:

> The newly introduced GENTLE_FAIR_SLEEPERS does not seem to have any
> effect without FAIR_SLEEPERS. Fix sysctl.sched_features to reflect
> this. Without this change, a user who sets GENTLE_FAIR_SLEEPERS
> without FAIR_SLEEPERS would assume gentle sleeper fairness which
> is not guaranteed.
> 
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
> 
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c
> +++ linux-2.6/kernel/sched.c
> @@ -761,10 +761,22 @@ sched_feat_write(struct file *filp, cons
>  		int len = strlen(sched_feat_names[i]);
>  
>  		if (strncmp(cmp, sched_feat_names[i], len) == 0) {
> -			if (neg)
> +			if (neg) {
>  				sysctl_sched_features &= ~(1UL << i);
> -			else
> +				/*
> +				 * GENTLE_FAIR_SLEEPERS have no effect without
> +				 * FAIR_SLEEPERS.
> +				 */
> +				if (strncmp(cmp, "FAIR_SLEEPERS",
> +					    strlen("FAIR_SLEEPERS")) == 0)
> +					sysctl_sched_features &= ~(1UL << i+1);
> +			} else {
>  				sysctl_sched_features |= (1UL << i);
> +				if (strncmp(cmp, "GENTLE_FAIR_SLEEPERS",
> +					    strlen("GENTLE_FAIR_SLEEPERS"))
> +						== 0)
> +					sysctl_sched_features |= (1UL << i-1);
> +			}
>  			break;
>  		}
>  	}

There's a lot of other dependencies between scheduler features so it's 
possible to change it without it having an effect on the scheduler. 

sched_features is really a development/debugging facility, you have to 
know what you are doing.

Might be worth adding a comment to the feature definition place itself 
in the source - explain what it does and how it makes sense (and how it 
doesnt).

	Ingo

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Suresh Jayaraman]

On 12/04/2009 03:24 PM, Ingo Molnar wrote:
> 
> * Suresh Jayaraman <sjayaraman@suse.de> wrote:
> 
>> The newly introduced GENTLE_FAIR_SLEEPERS does not seem to have any
>> effect without FAIR_SLEEPERS. Fix sysctl.sched_features to reflect
>> this. Without this change, a user who sets GENTLE_FAIR_SLEEPERS
>> without FAIR_SLEEPERS would assume gentle sleeper fairness which
>> is not guaranteed.
>>
> 
> There's a lot of other dependencies between scheduler features so it's 
> possible to change it without it having an effect on the scheduler. 
> 
> sched_features is really a development/debugging facility, you have to 
> know what you are doing.

I think originally introduced as a development/debugging facility,
sched_features is slowly transforming into a viable tool for System
Administrators, by looking at the impact of turning on/off some of these
features on some workloads (especially non-desktop workloads). And I
think these benefits should be passed on to the end users perhaps in the
form of documentation.

> Might be worth adding a comment to the feature definition place itself 
> in the source - explain what it does and how it makes sense (and how it 
> doesnt).
> 

Yes it make more sense to make such changes as part of documentation
than a code enforcement. I'll try and collect some of the useful tuning
information.


Thanks,

-- 
Suresh Jayaraman

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Peter Zijlstra]

On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
> 
> I think originally introduced as a development/debugging facility,
> sched_features is slowly transforming into a viable tool for System
> Administrators, by looking at the impact of turning on/off some of these
> features on some workloads (especially non-desktop workloads). And I
> think these benefits should be passed on to the end users perhaps in the
> form of documentation. 

This is really not meant to be used in that context. Its purely a debug
feature, with knobs coming and going as we see fit.

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Peter Zijlstra]

On Fri, 2009-12-04 at 12:08 +0100, Peter Zijlstra wrote:
> On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
> > 
> > I think originally introduced as a development/debugging facility,
> > sched_features is slowly transforming into a viable tool for System
> > Administrators, by looking at the impact of turning on/off some of these
> > features on some workloads (especially non-desktop workloads). And I
> > think these benefits should be passed on to the end users perhaps in the
> > form of documentation. 
> 
> This is really not meant to be used in that context. Its purely a debug
> feature, with knobs coming and going as we see fit.

That is, if this is how people are using it, I'll actually consider
removing the whole interface and keep it as an out-of-tree debug patch.

If a workload isn't working with the defaults they need to report a bug,
after that we can ask them to twiddle with some knobs to see what it is
that is not working.

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Suresh Jayaraman]

On 12/04/2009 04:38 PM, Peter Zijlstra wrote:
> On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
>>
>> I think originally introduced as a development/debugging facility,
>> sched_features is slowly transforming into a viable tool for System
>> Administrators, by looking at the impact of turning on/off some of these
>> features on some workloads (especially non-desktop workloads). And I
>> think these benefits should be passed on to the end users perhaps in the
>> form of documentation. 
> 
> This is really not meant to be used in that context. Its purely a debug
> feature, with knobs coming and going as we see fit.
> 

Does this also mean these features should not impact any specific
workload much?

http://osdir.com/ml/linux-kernel/2009-09/msg03406.html
In the thread above Ingo mentions about a few features and my
understanding is that some of these might favour one type of workload
than other. Is this not true anymore?


Thanks,

-- 
Suresh Jayaraman

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Peter Zijlstra]

On Fri, 2009-12-04 at 17:12 +0530, Suresh Jayaraman wrote:
> On 12/04/2009 04:38 PM, Peter Zijlstra wrote:
> > On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
> >>
> >> I think originally introduced as a development/debugging facility,
> >> sched_features is slowly transforming into a viable tool for System
> >> Administrators, by looking at the impact of turning on/off some of these
> >> features on some workloads (especially non-desktop workloads). And I
> >> think these benefits should be passed on to the end users perhaps in the
> >> form of documentation. 
> > 
> > This is really not meant to be used in that context. Its purely a debug
> > feature, with knobs coming and going as we see fit.
> > 
> 
> Does this also mean these features should not impact any specific
> workload much?

How would that follow?

> http://osdir.com/ml/linux-kernel/2009-09/msg03406.html
> In the thread above Ingo mentions about a few features and my
> understanding is that some of these might favour one type of workload
> than other. Is this not true anymore?

Sure it is, everything is workload dependent, the posix SCHED_OTHER task
model just doesn't include much usable information.

But that does not justify promoting this to generic tunable. What if you
happen to want to run two different workloads on one machine?

Its a CONFIG_SCHED_DEBUG thing, its in /debug (i've got a patch lined up
to remove the sysctl interface already), and I'm not going to guarantee
any kind of stability in the feature set what so ever.

Furthermore, if your favourite workload doesn't work well, file a bug
report (preferably with reproducer, otherwise its pure guesswork).

The only reason to poke at it is debugging, full stop, no whining or .33
won't have the interface anymore, which would be sad because then
everybody will have to recompile their kernel to debug things.

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Suresh Jayaraman]

On 12/04/2009 05:38 PM, Peter Zijlstra wrote:
> On Fri, 2009-12-04 at 17:12 +0530, Suresh Jayaraman wrote:
>> On 12/04/2009 04:38 PM, Peter Zijlstra wrote:
>>> On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
>>>>
>>>> I think originally introduced as a development/debugging facility,
>>>> sched_features is slowly transforming into a viable tool for System
>>>> Administrators, by looking at the impact of turning on/off some of these
>>>> features on some workloads (especially non-desktop workloads). And I
>>>> think these benefits should be passed on to the end users perhaps in the
>>>> form of documentation. 
>>>
>>> This is really not meant to be used in that context. Its purely a debug
>>> feature, with knobs coming and going as we see fit.
>>>
>>
>> Does this also mean these features should not impact any specific
>> workload much?
> 
> How would that follow?
> 
>> http://osdir.com/ml/linux-kernel/2009-09/msg03406.html
>> In the thread above Ingo mentions about a few features and my
>> understanding is that some of these might favour one type of workload
>> than other. Is this not true anymore?
> 
> Sure it is, everything is workload dependent, the posix SCHED_OTHER task
> model just doesn't include much usable information.
> 
> But that does not justify promoting this to generic tunable. What if you
> happen to want to run two different workloads on one machine?

Ok, I understand.

> 
> Furthermore, if your favourite workload doesn't work well, file a bug
> report (preferably with reproducer, otherwise its pure guesswork).

Make sense.

> The only reason to poke at it is debugging, full stop, no whining or .33
> won't have the interface anymore, which would be sad because then
> everybody will have to recompile their kernel to debug things.
> 

The intention was to understand better (if at all there is anything
tunable, if yes document) and definitely not whining. Please don't kill it.


Thanks,

-- 
Suresh Jayaraman

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Mike Galbraith]

On Fri, 2009-12-04 at 17:12 +0530, Suresh Jayaraman wrote:
> On 12/04/2009 04:38 PM, Peter Zijlstra wrote:
> > On Fri, 2009-12-04 at 15:50 +0530, Suresh Jayaraman wrote:
> >>
> >> I think originally introduced as a development/debugging facility,
> >> sched_features is slowly transforming into a viable tool for System
> >> Administrators, by looking at the impact of turning on/off some of these
> >> features on some workloads (especially non-desktop workloads). And I
> >> think these benefits should be passed on to the end users perhaps in the
> >> form of documentation. 
> > 
> > This is really not meant to be used in that context. Its purely a debug
> > feature, with knobs coming and going as we see fit.
> > 
> 
> Does this also mean these features should not impact any specific
> workload much?

Not at all.  Most loads will be heavily affected by one or more
features.

	-Mike

Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

[Mike Galbraith]

On Fri, 2009-12-04 at 14:59 +0530, Suresh Jayaraman wrote:
> The newly introduced GENTLE_FAIR_SLEEPERS does not seem to have any
> effect without FAIR_SLEEPERS. Fix sysctl.sched_features to reflect
> this. Without this change, a user who sets GENTLE_FAIR_SLEEPERS
> without FAIR_SLEEPERS would assume gentle sleeper fairness which
> is not guaranteed.

IMHO, these dependencies belong in documentation, not enforcement code.

Same applies for LAST_BUDDY, meaningless without WAKEUP_PREEMPT.
SYNC_MORE, SYNC_LESS + AFFINE_WAKEUPS etc.

Bottom line for all button poking and knob turning is you'd better make
sure you know what button/knob does, and what other things may depend on
it before messing with it, lest you get a nasty surprise.

	-Mike