Hookup group-scheduler with task container infrastructure
PATCH 1/1 Hookup group-scheduler with task container infrastructure
Ingo and Andrew,
The cfs core has been enhanced since quite sometime now to
understand task-groups and provide fairness to such task-group. What was needed is an interface for the administrator to define task-groups and specify group "importance" in terms of its cpu share.
The patch below adds such an interface. Its based on the task containers feature in -mm tree. I request you to review and include the patch in your trees for more wider testing from community.
Note that the load balancer needs more work, esp to deal with cases like 2-groups on 4-cpus, one group has 3 tasks and other having 4 tasks. We are working on some ideas, but nothing to share in the form of a patch yet. I felt sending this patch out would help folks start testing the feature and also improve it.
--- include/linux/container_subsys.h | 5 init/Kconfig | 9 + kernel/sched.c | 311 +++++++++++++++++++++++++++++++++++++-- kernel/sched_fair.c | 3 4 files changed, 313 insertions(+), 15 deletions(-)
include/linux/container_subsys.h +5 -0
@@ -36,3 +36,8 @@ SUBSYS(mem_container)#endif /* */ +#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpuctlr) +#endif + +/* */
init/Kconfig +9 -0
@@ -326,6 +326,15 @@ config RESOURCE_COUNTERS infrastructure that works with containers depends on CONTAINERS +config FAIR_GROUP_SCHED + bool "Fair group scheduler" + depends on EXPERIMENTAL && CONTAINERS + help + This option enables you to group tasks and control CPU resource + allocation to such groups. + + Say N if unsure. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/sched.c +298 -13
338 lines not on this page · raw message
kernel/sched_fair.c +1 -2
@@ -798,8 +798,7 @@ static inline struct cfs_rq *group_cfs_r*/ static inline struct cfs_rq *cpu_cfs_rq (struct cfs_rq *cfs_rq,int this_cpu) { -/* A later patch will take group into account */ -return &cpu_rq (this_cpu)->cfs; +return cfs_rq->tg->cfs_rq[this_cpu]; }/* Iterate thr' all leaf cfs_rq's on a runqueue */
Signature · 2 lines
Regards, vatsa
include/linux/container_subsys.h SUBSYS(mem_container) · line 41
Quoted 3 lines · Srivatsa Vaddagiri
include/linux/container_subsys.h +3
-0+#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpuctlr) +#endif
cpuctl, cpuctrl, cpu_controller?
Quoted 6 lines · Jan Engelhardt
include/linux/container_subsys.h SUBSYS(mem_container) · line 41
Quoted 3 lines · Srivatsa Vaddagiri
include/linux/container_subsys.h +3
-0+#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpuctlr) +#endif cpuctl, cpuctrl,
cpu_controller?
*shrug* .. I used "cpuctlr" to mean "CPU Controller". Any other short names would do. From your list, cpuctl or cpuctrl both qualifies IMO!
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
Signature · 2 lines
Regards, vatsa
Quoted 6 lines · Srivatsa Vaddagiri
Quoted 1 line
cpuctl, cpuctrl,
cpu_controller?*shrug* .. I used "cpuctlr" to mean "CPU Controller". Any other short names would do. From your list, cpuctl or cpuctrl both qualifies IMO!
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
s/cptctlr/cpuctlr !
Signature · 2 lines
Regards, vatsa
Quoted 9 lines · Srivatsa Vaddagiri
Quoted 6 lines · Srivatsa Vaddagiri
Quoted 1 line
cpuctl, cpuctrl,
cpu_controller?*shrug* .. I used "cpuctlr" to mean "CPU Controller". Any other short names would do. From your list, cpuctl or cpuctrl both qualifies IMO!
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
s/cptctlr/cpuctlr !
Captain Controller to the rescue!
Quoted 8 lines · On Mon, 10 Sep 2007 22:53:34 +0530 Srivatsa Vaddagiri…
Quoted 6 lines · Jan Engelhardt
include/linux/container_subsys.h SUBSYS(mem_container) · line 41
Quoted 3 lines · Srivatsa Vaddagiri
include/linux/container_subsys.h +3
-0+#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpuctlr) +#endif cpuctl, cpuctrl,
cpu_controller?
Quoted 5 lines
*shrug* .. I used "cpuctlr" to mean "CPU Controller". Any other short names would do. From your list, cpuctl or cpuctrl both qualifies IMO!
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "cpu_controller", please. The extra typing is worth it ;)
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)
Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
--- include/linux/container_subsys.h | 5 init/Kconfig | 9 + kernel/sched.c | 311 +++++++++++++++++++++++++++++++++++++-- kernel/sched_fair.c | 3 4 files changed, 313 insertions(+), 15 deletions(-)
include/linux/container_subsys.h +5 -0
@@ -36,3 +36,8 @@ SUBSYS(mem_container)#endif /* */ +#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpu_controller) +#endif + +/* */
init/Kconfig +9 -0
@@ -326,6 +326,15 @@ config RESOURCE_COUNTERS infrastructure that works with containers depends on CONTAINERS +config FAIR_GROUP_SCHED + bool "Fair group scheduler" + depends on EXPERIMENTAL && CONTAINERS + help + This option enables you to group tasks and control CPU resource + allocation to such groups. + + Say N if unsure. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/sched.c +298 -13
338 lines not on this page · raw message
kernel/sched_fair.c +1 -2
@@ -798,8 +798,7 @@ static inline struct cfs_rq *group_cfs_r*/ static inline struct cfs_rq *cpu_cfs_rq (struct cfs_rq *cfs_rq,int this_cpu) { -/* A later patch will take group into account */ -return &cpu_rq (this_cpu)->cfs; +return cfs_rq->tg->cfs_rq[this_cpu]; }/* Iterate thr' all leaf cfs_rq's on a runqueue */
Signature · 2 lines
Regards, vatsa
Quoted 8 lines · Srivatsa Vaddagiri
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
[ ... ]
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 6958
Quoted 10 lines
kernel/sched.c +10
-0+/* change task's runqueue when it moves between groups */ +static void sched_move_task (struct container_subsys *ss,struct container *cont, +struct container *old_cont,struct task_struct *tsk) +{ +int on_rq; +unsigned long flags; +struct rq *rq; + + rq =task_rq_lock (tsk, &flags); +
I guess, update_rq_clock(rq) should be placed here.
humm... do you really need deactivate/activate_task() here? 'rq' and p->se.load.weight stay unchanged so enqueue/dequeue_task() would do a job, no?
Although, I might be missing (definitely, as this part is not completely clear to me yet) something important at this late hour :)
Quoted 18 lines
kernel/sched.c +18
-0+ on_rq = tsk->se.on_rq; +if (on_rq) +deactivate_task (rq, tsk,0 ); + +if (unlikely (rq->curr == tsk) && tsk->sched_class == &fair_sched_class) + tsk->sched_class->put_prev_task (rq, tsk); + +set_task_cfs_rq (tsk); + +if (on_rq) +activate_task (rq, tsk,0 ); + +if (unlikely (rq->curr == tsk) && tsk->sched_class == &fair_sched_class) + tsk->sched_class->set_curr_task (rq); + +task_rq_unlock (rq, &flags); +} +
Signature · 2 lines
Best regards, Dmitry Adamushko
Quoted 8 lines · Dmitry Adamushko
Review
Quoted 2 lines
(quoted diff) +2
-0+ rq = task_rq_lock(tsk, &flags); +I guess,
update_rq_clock(rq)should be placed here.humm... do you really need deactivate/activate_task() here? 'rq' and
p->se.load.weightstay unchanged so enqueue/dequeue_task() would do a job, no?
Good catch. Here's the updated patch.
--- include/linux/container_subsys.h | 6 init/Kconfig | 9 + kernel/sched.c | 312 +++++++++++++++++++++++++++++++++++++-- kernel/sched_fair.c | 3 4 files changed, 315 insertions(+), 15 deletions(-)
include/linux/container_subsys.h +6 -0
@@ -36,3 +36,9 @@ SUBSYS(mem_container)#endif /* */ + +#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpu) +#endif + +/* */
init/Kconfig +9 -0
@@ -326,6 +326,15 @@ config RESOURCE_COUNTERS infrastructure that works with containers depends on CONTAINERS +config FAIR_GROUP_SCHED + bool "Fair group scheduler" + depends on EXPERIMENTAL && CONTAINERS + help + This option enables you to group tasks and control CPU resource + allocation to such groups. + + Say N if unsure. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/sched.c +299 -13
339 lines not on this page · raw message
kernel/sched_fair.c +1 -2
@@ -798,8 +798,7 @@ static inline struct cfs_rq *group_cfs_r*/ static inline struct cfs_rq *cpu_cfs_rq (struct cfs_rq *cfs_rq,int this_cpu) { -/* A later patch will take group into account */ -return &cpu_rq (this_cpu)->cfs; +return cfs_rq->tg->cfs_rq[this_cpu]; }/* Iterate thr' all leaf cfs_rq's on a runqueue */
Signature · 2 lines
Regards, vatsa
Quoted 2 lines · Srivatsa Vaddagiri
[ ... ]
I guess, 'rq->curr == tsk' implies a task was on the 'rq' (before dequeueing) in this particular case. What's about a minor optimization like below (plus, let's make use of task_running()):
[ btw., real-time task can be also added to a container, right? I guess, it can be used at least for group-aware load-balancing of rt_tasks... otherwise, I guess, cpu-resource controlling is not that applicable to, say, SCHED_FIFO :-)
Anyway, to this goal rt_task should become aware of group-related bits of the 'struct sched_entity'... and at the moment, the code below is effectively just a 'nop' for them.. right? ]
/* change task's runqueue when it moves between groups */
static void sched_move_task(struct container_subsys *ss, struct container *cont,
struct container *old_cont, struct task_struct *tsk)
{
int on_rq, running;
unsigned long flags;
struct rq *rq;
rq = task_rq_lock(tsk, &flags);
update_rq_clock(rq);
running = task_running(rq, tsk);
on_rq = tsk->se.on_rq;
if (on_rq) {
dequeue_task(rq, tsk, 0);
if (unlikely(running) && tsk->sched_class == &fair_sched_class)
tsk->sched_class->put_prev_task(rq, tsk);
}
set_task_cfs_rq(tsk);
if (on_rq) {
enqueue_task(rq, tsk, 0);
if (unlikely(running) && tsk->sched_class == &fair_sched_class)
tsk->sched_class->set_curr_task(rq);
}
task_rq_unlock(rq, &flags);
}Quoted 4 lines
--
Regards,
vatsa
Signature · 2 lines
Best regards, Dmitry Adamushko
Quoted 3 lines · Dmitry Adamushko
I guess, '
rq->curr== tsk' implies a task was on the 'rq' (before dequeueing) in this particular case. What's about a minor optimization like below (plus, let's make use oftask_running()):
Hi Dmitry,
The modified code looks very good to me (and neater too!). Updated
patch below.
Quoted 1 line
[ btw., real-time task can be also added to a container, right?
I am not sure whether we want to allow that. I have been assuming that only SCHED_NORMAL tasks will be in containers.
Imagine two SCHED_FIFO tasks being put in two different containers -and- controlling cpu cycle allocation to both containers. Do we actually timeslice the two SCHED_FIFO tasks now that are across containers? Also we need a mechanism to choose a container first to schedule and then the real-time task in it (similar to how its being done in SCHED_NORMAL case). Which means we need one rt_rq per container per cpu.
Kirill, do you require real-time tasks to be present in containers?
Quoted 3 lines
I guess, it can be used at least for group-aware load-balancing of
rt_tasks... otherwise, I guess, cpu-resource controlling is not that applicable to, say,SCHED_FIFO:-)
Yes, if we were to allow SCHED_FIFO/RR tasks in containers, then the load balancer would need to spread rt-tasks of a container across CPUs to ensure low scheduling latencies for them.
Quoted 3 lines
Anyway, to this goal
rt_taskshould become aware of group-related bits of the 'structsched_entity'... and at the moment, the code below is effectively just a 'nop' for them.. right? ]
Yes. To emphasise this code doesn't support real-time tasks in a container, I am returning -ENOTSUP when trying to move a rt-task into a container.
Thanks for all your review!
Add interface to control cpu bandwidth allocation to task-groups.
--- include/linux/container_subsys.h | 6 init/Kconfig | 9 + kernel/sched.c | 330 +++++++++++++++++++++++++++++++++++++-- kernel/sched_fair.c | 3 4 files changed, 333 insertions(+), 15 deletions(-)
include/linux/container_subsys.h +6 -0
@@ -36,3 +36,9 @@ SUBSYS(mem_container)#endif /* */ + +#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpu) +#endif + +/* */
init/Kconfig +9 -0
@@ -326,6 +326,15 @@ config RESOURCE_COUNTERS infrastructure that works with containers depends on CONTAINERS +config FAIR_GROUP_SCHED + bool "Fair group scheduler" + depends on EXPERIMENTAL && CONTAINERS + help + This option enables you to group tasks and control CPU resource + allocation to such groups. + + Say N if unsure. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/sched.c +317 -13
357 lines not on this page · raw message
kernel/sched_fair.c +1 -2
@@ -798,8 +798,7 @@ static inline struct cfs_rq *group_cfs_r*/ static inline struct cfs_rq *cpu_cfs_rq (struct cfs_rq *cfs_rq,int this_cpu) { -/* A later patch will take group into account */ -return &cpu_rq (this_cpu)->cfs; +return cfs_rq->tg->cfs_rq[this_cpu]; }/* Iterate thr' all leaf cfs_rq's on a runqueue */
Signature · 2 lines
Regards, vatsa
Quoted 2 lines · Srivatsa Vaddagiri
Yes. To emphasise this code doesn't support real-time tasks in a container, I am returning
-ENOTSUPwhen trying to move a rt-task into a container.
s/ENOTSUP/EINVAL
Signature · 2 lines
Regards, vatsa
Hi Srivatsa,
please find a few more minor comments below.
Quoted 20 lines
[ ... ]
kernel/sched.c +19
-0+ +/* destroy runqueue etc associated with a task group */ +static void sched_destroy_group (struct container_subsys *ss, +struct container *cont) +{ +struct task_grp *tg =container_tg (cont); +struct cfs_rq *cfs_rq; +struct sched_entity *se; +int i; + +for_each_possible_cpu (i) { + cfs_rq = tg->cfs_rq[i]; +list_del_rcu (&cfs_rq->leaf_cfs_rq_list); + } + +/* wait for possible concurrent references to cfs_rqs complete */ +synchronize_sched (); + +/* now it should be safe to free those cfs_rqs */
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 6947
Quoted 10 lines
kernel/sched.c +10
-0+for_each_possible_cpu (i) { + cfs_rq = tg->cfs_rq[i]; +kfree (cfs_rq); + + se = tg->se[i]; +kfree (se); + } + +kfree (tg); +}
kfree(tg->cfs_rq) && kfree(tg->se) ?
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 6970
Quoted 13 lines
kernel/sched.c +13
-0+ +/* change task's runqueue when it moves between groups */ +static void sched_move_task (struct container_subsys *ss,struct container *cont, +struct container *old_cont,struct task_struct *tsk) +{ +int on_rq, running; +unsigned long flags; +struct rq *rq; + + rq =task_rq_lock (tsk, &flags); + +if (tsk->sched_class != &fair_sched_class) + goto done;
this check should be redundant now with sched_can_attach() in place.
Quoted 4 lines
kernel/sched.c +4
-0+static void set_se_shares (struct sched_entity *se,unsigned long shares) +{ +struct cfs_rq *cfs_rq = se->cfs_rq; +struct rq *rq = cfs_rq->rq;
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 7008
Quoted 10 lines
kernel/sched.c +10
-0+int on_rq; + +spin_lock_irq (&rq->lock); + + on_rq = se->on_rq; +if (on_rq) +__dequeue_entity (cfs_rq, se); + + se->load.weight = shares; + se->load.inv_weight =div64_64 ((1ULL <<32 ), shares);
A bit of nit-picking... are you sure, there is no need in non '__' versions of dequeue/enqueu() here (at least, for the sake of update_curr())? Although, I don't have -mm at hand at this very moment and original -rc4 (that I have at hand) doesn't already have 'se->load' at all... so will look later.
Quoted 5 lines
--
Regards,
vatsa
Signature · 2 lines
Best regards, Dmitry Adamushko
Quoted 4 lines · Dmitry Adamushko
Review
Quoted 2 lines
(quoted diff) +2
-0+ kfree(tg); +}
kfree(tg->cfs_rq)&&kfree(tg->se)?
oops, yes!
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (tsk->sched_class != &fair_sched_class) + goto done;this check should be redundant now with
sched_can_attach()in place.
well, there is remote possibility that task could have changed policies between sched_can_attach() and sched_move_task()! In the updated patch below, I have put some hooks to deal with that scenario elsewhere also (in rt_mutex_setprio and __setscheduler). Pls let me know if you see any issues with that.
Quoted 9 lines
Quoted 9 lines
(quoted diff) +9
-0+static void set_se_shares(struct sched_entity *se, unsigned long shares) +{ + struct cfs_rq *cfs_rq = se->cfs_rq; + struct rq *rq = cfs_rq->rq; + int on_rq; + + spin_lock_irq(&rq->lock); + + on_rq = se->on_rq;
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ if (on_rq) + __dequeue_entity(cfs_rq, se); + + se->load.weight = shares; + se->load.inv_weight = div64_64((1ULL<<32), shares);A bit of nit-picking... are you sure, there is no need in non '__' versions of dequeue/enqueu() here (at least, for the sake of
You are right, we need the dequeue/enqueue_entity here, as the sched entity load is changing in between the two function calls.
How does the updated patch look? (Thanks again for all your review so far!)
--
Add interface to control cpu bandwidth allocation to task-groups.
--- include/linux/container_subsys.h | 6 init/Kconfig | 9 + kernel/sched.c | 344 +++++++++++++++++++++++++++++++++++++-- kernel/sched_fair.c | 3 kernel/sched_idletask.c | 5 kernel/sched_rt.c | 5 6 files changed, 355 insertions(+), 17 deletions(-)
include/linux/container_subsys.h +6 -0
@@ -36,3 +36,9 @@ SUBSYS(mem_container)#endif /* */ + +#ifdef CONFIG_FAIR_GROUP_SCHED +SUBSYS(cpu) +#endif + +/* */
init/Kconfig +9 -0
@@ -326,6 +326,15 @@ config RESOURCE_COUNTERS infrastructure that works with containers depends on CONTAINERS +config FAIR_GROUP_SCHED + bool "Fair group scheduler" + depends on EXPERIMENTAL && CONTAINERS + help + This option enables you to group tasks and control CPU resource + allocation to such groups. + + Say N if unsure. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/sched.c +329 -15
397 lines not on this page · raw message
kernel/sched_fair.c +1 -2
@@ -798,8 +798,7 @@ static inline struct cfs_rq *group_cfs_r*/ static inline struct cfs_rq *cpu_cfs_rq (struct cfs_rq *cfs_rq,int this_cpu) { -/* A later patch will take group into account */ -return &cpu_rq (this_cpu)->cfs; +return cfs_rq->tg->cfs_rq[this_cpu]; }/* Iterate thr' all leaf cfs_rq's on a runqueue */
kernel/sched_idletask.c +5 -0
@@ -50,6 +50,10 @@ static void task_tick_idle(struct rq *rq { } +static void set_curr_task_idle (struct rq *rq) +{ +} +/* * Simple, special scheduling class for the per-CPU idle tasks: */
@@ -66,6 +70,7 @@ static struct sched_class idle_sched_cla .load_balance = load_balance_idle, + .set_curr_task = set_curr_task_idle, .task_tick = task_tick_idle,/* no .task_new for idle tasks */ };
kernel/sched_rt.c +5 -0
@@ -218,6 +218,10 @@ static void task_tick_rt(struct rq *rq, } } +static void set_curr_task_rt (struct rq *rq) +{ +} +static struct sched_class rt_sched_class__read_mostly = { .enqueue_task = enqueue_task_rt, .dequeue_task = dequeue_task_rt,
@@ -230,5 +234,6 @@ static struct sched_class rt_sched_class .load_balance = load_balance_rt, + .set_curr_task = set_curr_task_rt, .task_tick = task_tick_rt, };
Signature · 2 lines
Regards, vatsa
Quoted 1 line · Srivatsa Vaddagiri
Add interface to control cpu bandwidth allocation to task-groups.
btw., just in case it was not obvious, i'll repeat my older assessment of your patch: the general picture approach looks good to me and the code is upstream-worthy.
( suggestion: if you want more people to test it then you might want to
do some add-on "put all users into separate groups" patch and .config option - which could be tried without people having to know anything about container setup. )
Ingo
Quoted 6 lines · Ingo Molnar
Quoted 1 line · Srivatsa Vaddagiri
Add interface to control cpu bandwidth allocation to task-groups.
btw., just in case it was not obvious, i'll repeat my older assessment of your patch: the general picture approach looks good to me and the code is upstream-worthy.
Thanks for the feedback!
Quoted 4 lines
( suggestion: if you want more people to test it then you might want to
do some add-on "put all users into separate groups" patch and .config option - which could be tried without people having to know anything about container setup. )
I do want more people to test it and I agree that hooking onto user-id based groups is the best way to get that done. How do we implement that? I have two choices:
1. Do a kernel patch, as you suggest above, which defines task-groups based on user-id and hook that group definition with group scheduler. We need to provide some means for the admin to tune relative nice-value of each user (perhaps thr' sysctl?).
This user-id based grouping will have to be mutually exclusive with task-container based grouping. Hence we need to ensure that only one form of grouping is selected and not both at compile time.
2. Enable only one form of grouping, which is task-container based. Provide a user-space daemon (attached) which can automatically put tasks of different users in different task-containers. The daemon will need to be started at early boot-time. It can also be extended to support a configuration file (ex: inittab) where cpu allocation for different users are specified. The fact that daemon is managing to provide fair allocation to users should be transparent.
I hope that task-containers (aka cgroups) will go into 2.6.24, in which case the second option seems to be more attractive to me.
I will neverthless try to work out Option 1, just to see how it looks.
--
Regards,
vatsa
Quoted 2 lines · Srivatsa Vaddagiri
2. Enable only one form of grouping, which is task-container based. Provide a user-space daemon (attached) which can automatically put tasks of different
Oops ..forgot to attach. But I realize that I had already sent the source for the daemon here:
http://article.gmane.org/gmane.linux.kernel/553267
Signature · 2 lines
Regards, vatsa
Quoted 5 lines · Srivatsa Vaddagiri
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
as everyone seems to be in a quest for a better name... I think, the obvious one would be just 'group_sched'.
Signature · 2 lines
Best regards, Dmitry Adamushko
Quoted 10 lines · Dmitry Adamushko
Quoted 5 lines · Srivatsa Vaddagiri
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
as everyone seems to be in a quest for a better name... I think, the obvious one would be just '
group_sched'.
But "sched" on its own could refer to CPU scheduling, I/O scheduling, network scheduling, ...
And "group" is more or less implied by the fact that it's in the containers/control groups filesystem.
So "group_sched" isn't really all that informative. The name should definitely contain either "cpu" or "cfs".
Paul
Quoted 10 lines · Paul Menage
Quoted 8 lines · Dmitry Adamushko
Quoted 4 lines · Srivatsa Vaddagiri
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
as everyone seems to be in a quest for a better name... I think, the obvious one would be just '
group_sched'.
Quoted 5 lines
But "sched" on its own could refer to CPU scheduling, I/O scheduling, network scheduling, ...
And "group" is more or less implied by the fact that it's in the containers/control groups filesystem.
"control groups" is the name of your framework. right ?
Quoted 2 lines
So "
group_sched" isn't really all that informative. The name should definitely contain either "cpu" or "cfs".
"cfs" control group subsystem.
"cfs" looks good enough to identify the subsystem.
C.
Quoted 17 lines · On Tue, 11 Sep 2007 09:20:33 +0200 Cedric Le Goater wrote:
Quoted 10 lines · Paul Menage
Quoted 8 lines · Dmitry Adamushko
Quoted 4 lines · Srivatsa Vaddagiri
Quoted 2 lines · Andrew Morton
objection ;) "cpuctlr" isn't memorable. Kernel code is write-rarely, read-often. "
cpu_controller", please. The extra typing is worth it ;)Ok! Here's the modified patch (against 2.6.23-rc4-mm1).
as everyone seems to be in a quest for a better name... I think, the obvious one would be just '
group_sched'.Quoted 5 lines
But "sched" on its own could refer to CPU scheduling, I/O scheduling, network scheduling, ...
And "group" is more or less implied by the fact that it's in the containers/control groups filesystem.
Quoted 6 lines
"control groups" is the name of your framework. right ?
Quoted 2 lines
So "
group_sched" isn't really all that informative. The name should definitely contain either "cpu" or "cfs"."cfs" control group subsystem.
That looks odd, like it's a filesystem.
What does cfs really mean?
Quoted 1 line
"cfs" looks good enough to identify the subsystem.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 4 lines · Randy Dunlap
Quoted 1 line
"cfs" control group subsystem.
That looks odd, like it's a filesystem.
What does cfs really mean?
http://en.wikipedia.org/wiki/CFS
(scnr)
It misses the C...something Filesystem tho.
Jan
Quoted 2 lines · Randy Dunlap
That looks odd, like it's a filesystem.
What does cfs really mean?
cfs = completely fair scheduler :)
In this thread, we are talking of hooking the cfs cpu scheduler with the task-container framework in -mm tree, so that the scheduler can deal with groups of tasks rather than just tasks, while handling fairness of cpu allocation.
I agree "cfs" control subsystem does look odd a bit here. "cpu" control subsystem seems better.
Signature · 2 lines
Regards, vatsa
Quoted 6 lines · On Tue, 11 Sep 2007 21:21:19 +0530 Srivatsa Vaddagiri wrote:
Quoted 2 lines · Randy Dunlap
That looks odd, like it's a filesystem.
What does cfs really mean?cfs = completely fair scheduler :)
Quoted 7 lines
In this thread, we are talking of hooking the cfs cpu scheduler with the task-container framework in -mm tree, so that the scheduler can deal with groups of tasks rather than just tasks, while handling fairness of cpu allocation.
I agree "cfs" control subsystem does look odd a bit here. "cpu" control subsystem seems better.
Thanks. I agree that using "cpu" is better. I.e., don't tie it to a particular scheduler name. It would just need to change the next time we have a new scheduler. ;)
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 5 lines · Cedric Le Goater
Quoted 3 lines
And "group" is more or less implied by the fact that it's in the containers/control groups filesystem.
"control groups" is the name of your framework. right ?
That's the main contender for the new name, to replace "task containers" since there were enough objections. (Renamed patches coming out today, I hope).
Paul
Quoted 3 lines · Andrew Morton
Quoted 1 line
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
Kernel code is write-rarely, read-often.
I think you mean __read_mostly. :-)
Jan
Quoted 3 lines · Srivatsa Vaddagiri
Unless folks have strong objection to it, I prefer "cptctlr", the way it is.
By definition any container (about to be renamed control group) subsystem is some kind of "controller" so that bit seems a bit redundant.
Any reason not to just call it "cpu" or "cpu_sched"
Paul
Quoted 5 lines · Paul Menage
By definition any container (about to be renamed control group) subsystem is some kind of "controller" so that bit seems a bit redundant.
Any reason not to just call it "cpu" or "
cpu_sched"
Done (in the latest patch I sent a while back)!
Signature · 2 lines
Regards, vatsa
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 6864
Quoted 6 lines · On Mon, 10 Sep 2007 22:40:49 +0530 Srivatsa Vaddagiri…
kernel/sched.c +6
-0+ tg->cfs_rq =kzalloc (sizeof (cfs_rq) *num_possible_cpus (),GFP_KERNEL ); +if (!tg->cfs_rq) + goto err; + tg->se =kzalloc (sizeof (se) *num_possible_cpus (),GFP_KERNEL ); +if (!tg->se) + goto err;
Sorry for very lazy responce..
num_possible_cpus() just returns # of possible cpus. Then it will not return Max-cpu-id to be used. I think just use NR_CPUS here is an easy way.
Thanks,
-Kame
Quoted 11 lines · KAMEZAWA Hiroyuki
kernel/sched.c void set_curr_task(int cpu, struct task_ · line 6864
Quoted 6 lines · Srivatsa Vaddagiri
kernel/sched.c +6
-0+ tg->cfs_rq =kzalloc (sizeof (cfs_rq) *num_possible_cpus (),GFP_KERNEL ); +if (!tg->cfs_rq) + goto err; + tg->se =kzalloc (sizeof (se) *num_possible_cpus (),GFP_KERNEL ); +if (!tg->se) + goto err;Sorry for very lazy responce..
num_possible_cpus()just returns # of possible cpus. Then it will not return Max-cpu-id to be used. I think just useNR_CPUShere is an easy way.
Good catch! Yes, you are right, the pointer array needs to be NR_CPUS size.
On closer examination, I think I can simply use alloc_percpu() here. Will send a patch after some testing.
Signature · 2 lines
Regards, vatsa