This patchset focus on autogroup runtime enable/disable tuning
issue.
Below is my test when disable autogroup at runtime:
vanilla:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
1046 test1 20 0 25544 844 276 R 50.0 0.0 0:23.30 bash
1044 test2 20 0 25544 860 280 R 25.2 0.0 0:20.55 bash
1045 test2 20 0 25544 864 284 R 24.8 0.0 0:17.48 bash
patched:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
991 test1 20 0 25544 844 276 R 31.7 0.0 2:05.93 bash
992 test2 20 0 25544 848 276 R 31.7 0.0 1:16.84 bash
994 test2 20 0 25544 856 284 R 29.8 0.0 1:02.67 bash
---
Yong Zhang (5):
sched, autogroup, sysctl: use proc_dointvec_minmax instead.
sched: hide cfs_rq of autogroup if autogroup is disabled
sched, autogroup: stop going ahead if autogroup is disabled
sched: refactor task_group()
sched, autogroup: runtime enable/disable tuning fix
include/linux/sched.h | 3 +++
kernel/sched.c | 10 +++++++++-
kernel/sched_autogroup.c | 38 ++++++++++++++++++++++++++++++++++++++
kernel/sched_autogroup.h | 5 +++++
kernel/sched_debug.c | 6 ++++++
kernel/sysctl.c | 2 +-
6 files changed, 62 insertions(+), 2 deletions(-)
When autogroup is disabled, sched_debug will show
all of the cfs_rq of autogroup by name "cfs_rq[0]:"
Actually that doesn't make sense and not necessary
to be showed.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sched_debug.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index eb6cb8e..121e2a2 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -169,6 +169,12 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
struct sched_entity *last;
unsigned long flags;
+#ifdef CONFIG_SCHED_AUTOGROUP
+ int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+ if (task_group_is_autogroup(cfs_rq->tg) && !enabled)
+ return;
+#endif
#ifdef CONFIG_FAIR_GROUP_SCHED
SEQ_printf(m, "\ncfs_rq[%d]:%s\n", cpu, task_group_path(cfs_rq->tg));
#else
--
1.7.1
when autogroup is disable from the beginning,
sched_autogroup_create_attach()
autogroup_move_group() <== 1
sched_move_task() <== 2
task_move_group_fair()
set_task_rq()
task_group()
autogroup_task_group()
We go the whole path without doing anything useful.
Then stop going further if autogroup is disabled.
But there will be a race window between 1 and 2, in which
sysctl_sched_autogroup_enabled is enabled. This issue
will be toke by following patch.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sched_autogroup.c | 4 ++++
kernel/sched_autogroup.h | 5 +++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..137a096 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -161,11 +161,15 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
p->signal->autogroup = autogroup_kref_get(ag);
+ if (!ACCESS_ONCE(sysctl_sched_autogroup_enabled))
+ goto out;
+
t = p;
do {
sched_move_task(t);
} while_each_thread(p, t);
+out:
unlock_task_sighand(p, &flags);
autogroup_kref_put(prev);
}
diff --git a/kernel/sched_autogroup.h b/kernel/sched_autogroup.h
index 7b859ff..0557705 100644
--- a/kernel/sched_autogroup.h
+++ b/kernel/sched_autogroup.h
@@ -1,6 +1,11 @@
#ifdef CONFIG_SCHED_AUTOGROUP
struct autogroup {
+ /*
+ * reference doesn't mean how many thread attach to this
+ * autogroup now. It just stands for the number of task
+ * could use this autogroup.
+ */
struct kref kref;
struct task_group *tg;
struct rw_semaphore lock;
--
1.7.1
move cgroup-related task group finding to cgroup_task_group().
No function change.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sched.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..5c74f79 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -601,7 +601,7 @@ static inline int cpu_of(struct rq *rq)
* holds that lock for each task it moves into the cgroup. Therefore
* by holding that lock, we pin the task to the current cgroup.
*/
-static inline struct task_group *task_group(struct task_struct *p)
+static inline struct task_group *cgroup_task_group(struct task_struct *p)
{
struct task_group *tg;
struct cgroup_subsys_state *css;
@@ -613,6 +613,14 @@ static inline struct task_group *task_group(struct task_struct *p)
lockdep_is_held(&task_rq(p)->lock));
tg = container_of(css, struct task_group, css);
+ return tg;
+}
+
+static inline struct task_group *task_group(struct task_struct *p)
+{
+ struct task_group *tg;
+
+ tg = cgroup_task_group(p);
return autogroup_task_group(p, tg);
}
--
1.7.1
Now enable/disable autogroup at runtime is just a flag,
it doesn't take effect on current process of the system.
when disable autogroup:
vanilla:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
1046 test1 20 0 25544 844 276 R 50.0 0.0 0:23.30 bash
1044 test2 20 0 25544 860 280 R 25.2 0.0 0:20.55 bash
1045 test2 20 0 25544 864 284 R 24.8 0.0 0:17.48 bash
patched:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
991 test1 20 0 25544 844 276 R 31.7 0.0 2:05.93 bash
992 test2 20 0 25544 848 276 R 31.7 0.0 1:16.84 bash
994 test2 20 0 25544 856 284 R 29.8 0.0 1:02.67 bash
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
include/linux/sched.h | 3 +++
kernel/sched_autogroup.c | 34 ++++++++++++++++++++++++++++++++++
kernel/sysctl.c | 2 +-
3 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..00791ce 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1949,6 +1949,9 @@ extern unsigned int sysctl_sched_compat_yield;
#ifdef CONFIG_SCHED_AUTOGROUP
extern unsigned int sysctl_sched_autogroup_enabled;
+extern int sysctl_sched_autogroup_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos);
extern void sched_autogroup_create_attach(struct task_struct *p);
extern void sched_autogroup_detach(struct task_struct *p);
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 137a096..b1ad946 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -211,6 +211,40 @@ static int __init setup_autogroup(char *str)
__setup("noautogroup", setup_autogroup);
+static DEFINE_MUTEX(autogroup_sysctl_mutex);
+
+int sysctl_sched_autogroup_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos)
+{
+ int old_value, ret;
+ struct task_struct *p, *n;
+
+ mutex_lock(&autogroup_sysctl_mutex);
+
+ old_value = sysctl_sched_autogroup_enabled;
+ ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+
+ if (ret || !write || (old_value == sysctl_sched_autogroup_enabled))
+ goto out_unlock;
+
+ read_lock_irq(&tasklist_lock);
+ rcu_read_lock();
+
+ do_each_thread(n, p) {
+ if (cgroup_task_group(p) == &root_task_group)
+ sched_move_task(p);
+ } while_each_thread(n, p);
+
+ rcu_read_unlock();
+ read_unlock_irq(&tasklist_lock);
+
+out_unlock:
+ mutex_unlock(&autogroup_sysctl_mutex);
+
+ return ret;
+}
+
#ifdef CONFIG_PROC_FS
int proc_sched_autogroup_set_nice(struct task_struct *p, int *nice)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 22e07ee..2b12c65 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -374,7 +374,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec_minmax,
+ .proc_handler = sysctl_sched_autogroup_handler,
.extra1 = &zero,
.extra2 = &one,
},
--
1.7.1
sched_autogroup_enabled has min/max value, proc_dointvec_minmax()
is be used for this case.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sysctl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 0f1bd83..22e07ee 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -374,7 +374,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
--
1.7.1
On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> Now enable/disable autogroup at runtime is just a flag,
> it doesn't take effect on current process of the system.
Yup, they only move upon migration, which can take up to forever. Linus
didn't like the idea of an active on/off switch, so it got whacked.
-Mike
On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > Now enable/disable autogroup at runtime is just a flag,
> > it doesn't take effect on current process of the system.
>
> Yup, they only move upon migration, which can take up to forever.
Yeah. so on UP, the user can have autogroup forever or not, and
sched_autogroup_enable is a nop.
> Linus
> didn't like the idea of an active on/off switch, so it got whacked.
Seems I missed that very thread.
Apart from runtime enable/disable issue, I think patch1~3 could
still be applied beause they really fix some problem or bring
some improvement.
Thanks,
Yong
On Sun, 2011-02-20 at 22:10 +0800, Yong Zhang wrote:
> On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> > On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > > Now enable/disable autogroup at runtime is just a flag,
> > > it doesn't take effect on current process of the system.
> >
> > Yup, they only move upon migration, which can take up to forever.
>
> Yeah. so on UP, the user can have autogroup forever or not, and
> sched_autogroup_enable is a nop.
Yup, and that's the best argument for doing the active switch.
> Apart from runtime enable/disable issue, I think patch1~3 could
> still be applied beause they really fix some problem or bring
> some improvement.
Yeah.
Your whole series looked fine to me at a glance (sunday;), with the
exception of #2, that one is maybe iffy, depending on point of view.
It's iffy only in that the proc interface is there to show the group
(whether used or not, it's not about tasks really, just group), so the
user can easily twiddle it if he so desires, which has nada to do with
the group being active or not.
That said, I'd agree to whacking the proc interface wholesale in a
heartbeat. It's really a wart. If it were frequently used, it'd be
indicative of failure for the whole concept.
-Mike
On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
> On Sun, 2011-02-20 at 22:10 +0800, Yong Zhang wrote:
> > On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> > > On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > > > Now enable/disable autogroup at runtime is just a flag,
> > > > it doesn't take effect on current process of the system.
> > >
> > > Yup, they only move upon migration, which can take up to forever.
> >
> > Yeah. so on UP, the user can have autogroup forever or not, and
> > sched_autogroup_enable is a nop.
>
> Yup, and that's the best argument for doing the active switch.
>
> > Apart from runtime enable/disable issue, I think patch1~3 could
> > still be applied beause they really fix some problem or bring
> > some improvement.
>
> Yeah.
>
> Your whole series looked fine to me at a glance (sunday;), with the
> exception of #2, that one is maybe iffy, depending on point of view.
Hm, my mind wandered off to different file somewhere along the line :)
-Mike
On Mon, Feb 21, 2011 at 11:13 AM, Mike Galbraith <[email protected]> wrote:
>> Your whole series looked fine to me at a glance (sunday;), with the
>> exception of #2, that one is maybe iffy, depending on point of view.
>
> Hm, my mind wandered off to different file somewhere along the line :)
Never mind :)
If we relax the runtime autogroup switch, maybe patch-2 is a little
overkill, because after autogroup is disabled there still could have
some tasks staying on it's autogroup.
So if we wan to keep current autogroup switch behavior, I think
we can make autogroup_path() independent on
sysctl_sched_autogroup_enabled, thus user will not be confused
by lots of groups named "cfs_rq[*]:"
But if this patch is accepted, IMHO, patch-2 is ok.
Thanks,
Yong
--
Only stand for myself
On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
>
> Your whole series looked fine to me at a glance (sunday;), with the
> exception of #2, that one is maybe iffy, depending on point of view.
Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
autogroup it should display it.
As to 5, I really don't care, Linus did seem to care initially, don't
know how he feels about it now.
* Peter Zijlstra <[email protected]> wrote:
> On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
> >
> > Your whole series looked fine to me at a glance (sunday;), with the
> > exception of #2, that one is maybe iffy, depending on point of view.
>
> Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
> autogroup it should display it.
>
> As to 5, I really don't care, Linus did seem to care initially, don't
> know how he feels about it now.
I've Cc:-ed him.
Since Yong Zhang has noticed the difference and has written and tested the patch we
should consider it i think, it's a QoI improvement. The knob is root-only, right?
Thanks,
Ingo
On Tue, Feb 22, 2011 at 12:36 AM, Peter Zijlstra <[email protected]> wrote:
> On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
>>
>> Your whole series looked fine to me at a glance (sunday;), with the
>> exception of #2, that one is maybe iffy, depending on point of view.
>
> Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
> autogroup it should display it.
So how about replacing patch-2 by below one for now?
---
From: Yong Zhang <[email protected]>
Subject: [PATCH] sched, autogroup: always show autogroup name in sched_debug
When autogroup is disabled, there will be lots of group
named "cfs_rq[cpu]:", thus will lead to confusion for
who read it.
And for now, autogroup runtime disable/enable will
not take effect immediately on current live processes,
so there maybe still have some processes attaching to
its autogroup.
So show autogroup name always. But for the root group,
its name will dance between "cfs_rq[cpu]:" and
"cfs_rq[cpu]:/autogroup-0" according to
sysctl_sched_autogroup_enabled.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sched_autogroup.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 64919dc..6114e3e 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -301,7 +301,7 @@ static inline int autogroup_path(struct task_group
*tg, char *buf, int buflen)
{
int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
- if (!enabled || !tg->autogroup)
+ if (!tg->autogroup || (!tg->autogroup->id && !enabled))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
--
1.7.1
--
Only stand for myself
On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> So show autogroup name always. But for the root group,
> its name will dance between "cfs_rq[cpu]:" and
> "cfs_rq[cpu]:/autogroup-0" according to
> sysctl_sched_autogroup_enabled.
Would it make sense to force the root group to always be a !autogroup?
On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > So show autogroup name always. But for the root group,
> > its name will dance between "cfs_rq[cpu]:" and
> > "cfs_rq[cpu]:/autogroup-0" according to
> > sysctl_sched_autogroup_enabled.
>
> Would it make sense to force the root group to always be a !autogroup?
Hmmm, I think it's ok.
---
From: Yong Zhang <[email protected]>
Subject: [PATCH] sched, autogroup: always show autogroup name in sched_debug
When autogroup is disabled, there will be lots of group
named "cfs_rq[cpu]:", thus will lead to confusion for
who read it.
And for now, autogroup runtime disable/enable will
not take effect immediately on current live processes,
so there maybe still have some processes attaching to
its autogroup.
So show autogroup name always. But for the root group,
its name will always be "cfs_rq[cpu]:/".
Signed-off-by: Yong Zhang <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
---
kernel/sched_autogroup.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..5b9a317 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -258,9 +258,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!tg->autogroup || !tg->autogroup->id)
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
--
1.7.1
On Tue, 2011-02-22 at 20:10 +0800, Yong Zhang wrote:
> On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> > On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > > So show autogroup name always. But for the root group,
> > > its name will dance between "cfs_rq[cpu]:" and
> > > "cfs_rq[cpu]:/autogroup-0" according to
> > > sysctl_sched_autogroup_enabled.
> >
> > Would it make sense to force the root group to always be a !autogroup?
>
> Hmmm, I think it's ok.
Yup, it's not an autogroup, so let's stop claiming it.
sched, autogroup: stop claiming ownership of the root task group.
Disown it, and only display autogroup association if one exists.
Signed-off-by: Mike Galbraith <[email protected]>
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..506faa0 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
static void __init autogroup_init(struct task_struct *init_task)
{
autogroup_default.tg = &root_task_group;
- root_task_group.autogroup = &autogroup_default;
kref_init(&autogroup_default.kref);
init_rwsem(&autogroup_default.lock);
init_task->signal->autogroup = &autogroup_default;
@@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
static inline bool task_group_is_autogroup(struct task_group *tg)
{
- return tg != &root_task_group && tg->autogroup;
+ return !!tg->autogroup;
}
static inline struct task_group *
@@ -247,10 +246,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
{
struct autogroup *ag = autogroup_task_get(p);
+ if (!task_group_is_autogroup(ag->tg))
+ goto out;
+
down_read(&ag->lock);
seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
up_read(&ag->lock);
+out:
autogroup_kref_put(ag);
}
#endif /* CONFIG_PROC_FS */
@@ -258,9 +261,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!task_group_is_autogroup(tg))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
On Tue, Feb 22, 2011 at 03:02:00PM +0100, Mike Galbraith wrote:
> On Tue, 2011-02-22 at 20:10 +0800, Yong Zhang wrote:
> > On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> > > On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > > > So show autogroup name always. But for the root group,
> > > > its name will dance between "cfs_rq[cpu]:" and
> > > > "cfs_rq[cpu]:/autogroup-0" according to
> > > > sysctl_sched_autogroup_enabled.
> > >
> > > Would it make sense to force the root group to always be a !autogroup?
> >
> > Hmmm, I think it's ok.
>
> Yup, it's not an autogroup, so let's stop claiming it.
>
> sched, autogroup: stop claiming ownership of the root task group.
>
> Disown it, and only display autogroup association if one exists.
>
> Signed-off-by: Mike Galbraith <[email protected]>
Ah, I like this one better :)
Actually I misunderstanded what Peter mean.
Reviewed-by: Yong Zhang <[email protected]>
Thanks,
Yong
>
> diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
> index 9fb6562..506faa0 100644
> --- a/kernel/sched_autogroup.c
> +++ b/kernel/sched_autogroup.c
> @@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
> static void __init autogroup_init(struct task_struct *init_task)
> {
> autogroup_default.tg = &root_task_group;
> - root_task_group.autogroup = &autogroup_default;
> kref_init(&autogroup_default.kref);
> init_rwsem(&autogroup_default.lock);
> init_task->signal->autogroup = &autogroup_default;
> @@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
>
> static inline bool task_group_is_autogroup(struct task_group *tg)
> {
> - return tg != &root_task_group && tg->autogroup;
> + return !!tg->autogroup;
> }
>
> static inline struct task_group *
> @@ -247,10 +246,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
> {
> struct autogroup *ag = autogroup_task_get(p);
>
> + if (!task_group_is_autogroup(ag->tg))
> + goto out;
> +
> down_read(&ag->lock);
> seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
> up_read(&ag->lock);
>
> +out:
> autogroup_kref_put(ag);
> }
> #endif /* CONFIG_PROC_FS */
> @@ -258,9 +261,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
> #ifdef CONFIG_SCHED_DEBUG
> static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
> {
> - int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
> -
> - if (!enabled || !tg->autogroup)
> + if (!task_group_is_autogroup(tg))
> return 0;
>
> return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
>
Commit-ID: 1747b21fecbfb63fbf6b9624e8b92707960d5a97
Gitweb: http://git.kernel.org/tip/1747b21fecbfb63fbf6b9624e8b92707960d5a97
Author: Yong Zhang <[email protected]>
AuthorDate: Sun, 20 Feb 2011 15:08:12 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 23 Feb 2011 11:33:58 +0100
sched, autogroup, sysctl: Use proc_dointvec_minmax() instead
sched_autogroup_enabled has min/max value, proc_dointvec_minmax() is
be used for this case.
Signed-off-by: Yong Zhang <[email protected]>
Cc: Mike Galbraith <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sysctl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index cb7c830..7b5eead 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -367,7 +367,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
Commit-ID: 800d4d30c8f20bd728e5741a3b77c4859a613f7c
Gitweb: http://git.kernel.org/tip/800d4d30c8f20bd728e5741a3b77c4859a613f7c
Author: Yong Zhang <[email protected]>
AuthorDate: Sun, 20 Feb 2011 15:08:14 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 23 Feb 2011 11:33:59 +0100
sched, autogroup: Stop going ahead if autogroup is disabled
when autogroup is disable from the beginning,
sched_autogroup_create_attach()
autogroup_move_group() <== 1
sched_move_task() <== 2
task_move_group_fair()
set_task_rq()
task_group()
autogroup_task_group()
We go the whole path without doing anything useful.
Then stop going further if autogroup is disabled.
But there will be a race window between 1 and 2, in which
sysctl_sched_autogroup_enabled is enabled. This issue
will be toke by following patch.
Signed-off-by: Yong Zhang <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched_autogroup.c | 4 ++++
kernel/sched_autogroup.h | 5 +++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..137a096 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -161,11 +161,15 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
p->signal->autogroup = autogroup_kref_get(ag);
+ if (!ACCESS_ONCE(sysctl_sched_autogroup_enabled))
+ goto out;
+
t = p;
do {
sched_move_task(t);
} while_each_thread(p, t);
+out:
unlock_task_sighand(p, &flags);
autogroup_kref_put(prev);
}
diff --git a/kernel/sched_autogroup.h b/kernel/sched_autogroup.h
index 7b859ff..0557705 100644
--- a/kernel/sched_autogroup.h
+++ b/kernel/sched_autogroup.h
@@ -1,6 +1,11 @@
#ifdef CONFIG_SCHED_AUTOGROUP
struct autogroup {
+ /*
+ * reference doesn't mean how many thread attach to this
+ * autogroup now. It just stands for the number of task
+ * could use this autogroup.
+ */
struct kref kref;
struct task_group *tg;
struct rw_semaphore lock;
Commit-ID: 511f67a5997c4967c69a3961e2fc9f04d8d244ac
Gitweb: http://git.kernel.org/tip/511f67a5997c4967c69a3961e2fc9f04d8d244ac
Author: Mike Galbraith <[email protected]>
AuthorDate: Tue, 22 Feb 2011 15:02:00 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 23 Feb 2011 11:34:03 +0100
sched, autogroup: Stop claiming ownership of the root task group
Disown it, and only display autogroup association if one exists.
Signed-off-by: Mike Galbraith <[email protected]>
Reviewed-by: Yong Zhang <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched_autogroup.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 137a096..5946ac5 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
static void __init autogroup_init(struct task_struct *init_task)
{
autogroup_default.tg = &root_task_group;
- root_task_group.autogroup = &autogroup_default;
kref_init(&autogroup_default.kref);
init_rwsem(&autogroup_default.lock);
init_task->signal->autogroup = &autogroup_default;
@@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
static inline bool task_group_is_autogroup(struct task_group *tg)
{
- return tg != &root_task_group && tg->autogroup;
+ return !!tg->autogroup;
}
static inline struct task_group *
@@ -251,10 +250,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
{
struct autogroup *ag = autogroup_task_get(p);
+ if (!task_group_is_autogroup(ag->tg))
+ goto out;
+
down_read(&ag->lock);
seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
up_read(&ag->lock);
+out:
autogroup_kref_put(ag);
}
#endif /* CONFIG_PROC_FS */
@@ -262,9 +265,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!task_group_is_autogroup(tg))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);