[PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[Clark Williams]

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


Add a /proc/sys/kernel scheduler knob named sched_rr_timeslice_ms
that allows global changing of the SCHED_RR timeslice value. User
visable value is in milliseconds but is stored as jiffies.  Setting
to 0 (zero) resets to the default (currently 100ms).

Signed-off-by: Clark Williams <williams@redhat.com>
---
 include/linux/sched/sysctl.h |  5 +++++
 kernel/sched/core.c          | 19 +++++++++++++++++++
 kernel/sched/rt.c            |  6 ++++--
 kernel/sysctl.c              |  8 ++++++++
 4 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h
index 912adab..fda131f 100644
--- a/include/linux/sched/sysctl.h
+++ b/include/linux/sched/sysctl.h
@@ -10,6 +10,11 @@
  */
 #define RR_TIMESLICE		(100 * HZ / 1000)
 
+extern int sched_rr_timeslice;
+extern int sched_rr_handler(struct ctl_table *table, int write,
+		void __user *buffer, size_t *lenp,
+		loff_t *ppos);
+
 /*
  *  control realtime throttling:
  *
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 26058d0..1c39c33 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7508,6 +7508,25 @@ static int sched_rt_global_constraints(void)
 }
 #endif /* CONFIG_RT_GROUP_SCHED */
 
+int sched_rr_handler(struct ctl_table *table, int write,
+		void __user *buffer, size_t *lenp,
+		loff_t *ppos)
+{
+	int ret;
+	static DEFINE_MUTEX(mutex);
+
+	mutex_lock(&mutex);
+	ret = proc_dointvec(table, write, buffer, lenp, ppos);
+	/* make sure that internally we keep jiffies */
+	/* also, writing zero resets timeslice to default */
+	if (!ret && write) {
+		sched_rr_timeslice = sched_rr_timeslice <= 0 ?
+			RR_TIMESLICE :
msecs_to_jiffies(sched_rr_timeslice);
+	}
+	mutex_unlock(&mutex);
+	return ret;
+}
+
 int sched_rt_handler(struct ctl_table *table, int write,
 		void __user *buffer, size_t *lenp,
 		loff_t *ppos)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 7c4a007..0fd9d42 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -7,6 +7,8 @@
 
 #include <linux/slab.h>
 
+int sched_rr_timeslice = RR_TIMESLICE;
+
 static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int
overrun); 
 struct rt_bandwidth def_rt_bandwidth;
@@ -2017,7 +2019,7 @@ static void task_tick_rt(struct rq *rq, struct
task_struct *p, int queued) if (--p->rt.time_slice)
 		return;
 
-	p->rt.time_slice = RR_TIMESLICE;
+	p->rt.time_slice = sched_rr_timeslice;
 
 	/*
 	 * Requeue to the end of queue if we (and all of our
ancestors) are the @@ -2048,7 +2050,7 @@ static unsigned int
get_rr_interval_rt(struct rq *rq, struct task_struct *task)
 	 * Time slice is 0 for SCHED_FIFO tasks
 	 */
 	if (task->policy == SCHED_RR)
-		return RR_TIMESLICE;
+		return sched_rr_timeslice;
 	else
 		return 0;
 }
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index c88878d..1eabf86 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -403,6 +403,14 @@ static struct ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= sched_rt_handler,
 	},
+	{
+		.procname	= "sched_rr_timeslice_ms",
+		.data		= &sched_rr_timeslice,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= sched_rr_handler,
+	},
+
 #ifdef CONFIG_SCHED_AUTOGROUP
 	{
 		.procname	= "sched_autogroup_enabled",
-- 
1.7.11.7


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[Ingo Molnar]

* Clark Williams <williams@redhat.com> wrote:

> 
> Add a /proc/sys/kernel scheduler knob named sched_rr_timeslice_ms
> that allows global changing of the SCHED_RR timeslice value. User
> visable value is in milliseconds but is stored as jiffies.  Setting
> to 0 (zero) resets to the default (currently 100ms).
> 
> Signed-off-by: Clark Williams <williams@redhat.com>
> ---
>  include/linux/sched/sysctl.h |  5 +++++
>  kernel/sched/core.c          | 19 +++++++++++++++++++
>  kernel/sched/rt.c            |  6 ++++--
>  kernel/sysctl.c              |  8 ++++++++
>  4 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h
> index 912adab..fda131f 100644
> --- a/include/linux/sched/sysctl.h
> +++ b/include/linux/sched/sysctl.h
> @@ -10,6 +10,11 @@
>   */
>  #define RR_TIMESLICE		(100 * HZ / 1000)
>  
> +extern int sched_rr_timeslice;
> +extern int sched_rr_handler(struct ctl_table *table, int write,
> +		void __user *buffer, size_t *lenp,
> +		loff_t *ppos);
> +
>  /*
>   *  control realtime throttling:
>   *
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 26058d0..1c39c33 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7508,6 +7508,25 @@ static int sched_rt_global_constraints(void)
>  }
>  #endif /* CONFIG_RT_GROUP_SCHED */
>  
> +int sched_rr_handler(struct ctl_table *table, int write,
> +		void __user *buffer, size_t *lenp,
> +		loff_t *ppos)
> +{
> +	int ret;
> +	static DEFINE_MUTEX(mutex);
> +
> +	mutex_lock(&mutex);
> +	ret = proc_dointvec(table, write, buffer, lenp, ppos);
> +	/* make sure that internally we keep jiffies */
> +	/* also, writing zero resets timeslice to default */
> +	if (!ret && write) {
> +		sched_rr_timeslice = sched_rr_timeslice <= 0 ?
> +			RR_TIMESLICE :
> msecs_to_jiffies(sched_rr_timeslice);

hm, this patch is whitespace damaged. (line wrapped)

The first patch looks good, I've applied it.

Thanks,

	Ingo

Re: [PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> The first patch looks good, I've applied it.

I've skipped this one after all, as it sometimes generates:

include/linux/sched/sysctl.h:108:41: warning: ‘struct signal_struct’ declared inside parameter list [enabled by default]

Sometimes doesn't built kernel/sched/auto_group.c. So those bits 
probably want to move into a separate sched/auto_group.h header 
or so.

But, more fundamentally, I think sched.h should not include the 
new sched/sysctl.h file. The whole point is to move out bits 
from sched.h and decouple them from sched.h - to shrink sched.h 
in the long run.

Thanks,

	Ingo

Re: [PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[Clark Williams]

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

On Sun, 3 Feb 2013 19:57:09 +0100
Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > The first patch looks good, I've applied it.
> 
> I've skipped this one after all, as it sometimes generates:
> 
> include/linux/sched/sysctl.h:108:41: warning: ‘struct signal_struct’ declared inside parameter list [enabled by default]
> 
> Sometimes doesn't built kernel/sched/auto_group.c. So those bits 
> probably want to move into a separate sched/auto_group.h header 
> or so.
> 
> But, more fundamentally, I think sched.h should not include the 
> new sched/sysctl.h file. The whole point is to move out bits 
> from sched.h and decouple them from sched.h - to shrink sched.h 
> in the long run.
> 
> Thanks,
> 
> 	Ingo

Ahh, you wanted to not only reduce the size of sched.h you want to
reduce the amount of code it brings in. 

I'll go back to the drawing board. I'll pull the relevant code from
sysctl.h into auto_group.h, then I'll remove the include in sched.h.
Then I'll track down all the build failures from the above :).

Clark

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv2 2/2] sched/rt: add a tuning knob to allow changing SCHED_RR timeslice

[Ingo Molnar]

* Clark Williams <williams@redhat.com> wrote:

> On Sun, 3 Feb 2013 19:57:09 +0100
> Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > > The first patch looks good, I've applied it.
> > 
> > I've skipped this one after all, as it sometimes generates:
> > 
> > include/linux/sched/sysctl.h:108:41: warning: ???struct signal_struct??? declared inside parameter list [enabled by default]
> > 
> > Sometimes doesn't built kernel/sched/auto_group.c. So those bits 
> > probably want to move into a separate sched/auto_group.h header 
> > or so.
> > 
> > But, more fundamentally, I think sched.h should not include the 
> > new sched/sysctl.h file. The whole point is to move out bits 
> > from sched.h and decouple them from sched.h - to shrink sched.h 
> > in the long run.
> > 
> > Thanks,
> > 
> > 	Ingo
> 
> Ahh, you wanted to not only reduce the size of sched.h you 
> want to reduce the amount of code it brings in.

Yeah - I wanted your series to kick-start the ambitious project 
of splitting up the mega-monolithic sched.h include file - 
without anyone noticing. Now my plan is busted.

Many good people scared away from this daunting project in the 
past one or two decades, but they were chicken!

> I'll go back to the drawing board. I'll pull the relevant code 
> from sysctl.h into auto_group.h, then I'll remove the include 
> in sched.h. Then I'll track down all the build failures from 
> the above :).

As long as a few common config combinations work fine I can help 
out with tracking down weird config failures and can also fix 
them - in fact I almost managed to fix your previous series when 
I noticed that include/sched/ is still included in sched.h. (the 
price I paid for not looking closely enough before applying the 
patches.)

Thanks,

	Ingo