2009-12-04 09:29:20

by Suresh Jayaraman

[permalink] [raw]
Subject: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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 <[email protected]>
---

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;
}
}


2009-12-04 09:54:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency


* Suresh Jayaraman <[email protected]> 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 <[email protected]>
> ---
>
> 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

2009-12-04 10:06:25

by Mike Galbraith

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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

2009-12-04 10:20:09

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

On 12/04/2009 03:24 PM, Ingo Molnar wrote:
>
> * Suresh Jayaraman <[email protected]> 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

2009-12-04 11:08:42

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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.


2009-12-04 11:12:22

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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.


2009-12-04 11:42:20

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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

2009-12-04 12:08:41

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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.

2009-12-04 13:12:43

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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

2009-12-04 13:13:00

by Mike Galbraith

[permalink] [raw]
Subject: Re: [RFC][PATCH] sched: fix GENTLE_FAIR_SLEEPERS dependency

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