Messages in this thread | | | From | Paul Menage <> | Date | Mon, 25 Jul 2011 18:17:08 -0700 | Subject | Re: [PATCH 7/7] cgroups: Add a max number of tasks subsystem |
| |
On Mon, Jul 11, 2011 at 7:15 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote: > /* */ > + > +#ifdef CONFIG_CGROUP_MAX_TASKS > +SUBSYS(max_tasks) > +#endif
I think I'd be inclined to make the naming slightly (IMO) cleaner: call the subsystem 'tasks' and the files 'limit' and 'count' (or maybe 'usage' to be more in line with memcg). > > +config CGROUP_MAX_TASKS > + bool "Control max number of tasks in a cgroup" > + depends on RESOURCE_COUNTERS > + help > + This option let the user to set up an upper bound allowed number > + of tasks. > +
Needs to depend on CGROUPS too?
> + > + > +struct task_counter { > + struct res_counter res; > + struct cgroup_subsys_state css; > +};
All other CSS structures put the "css" field as the first thing in the structure. Not sure that anything relies on that, but consistency can't hurt and it probably makes the code fractionally smaller since the translation between CSS and task_counter becomes a no-op.
> +static void task_counter_exit(struct cgroup_subsys *ss, struct cgroup *cgrp, > + struct cgroup *old_cgrp, struct task_struct *task) > +{ > + struct task_counter *cnt = cgroup_task_counter(old_cgrp); > + > + if (cnt != &root_counter) > + res_counter_uncharge_until(&cnt->res, &root_counter.res, 1); > +}
Do we even need the root_counter to be exposed in any way? Why not make children of the root cgroup just have a NULL parent res_counter?
You'll still need a task_counter object so that the cgroups framework has a CSS object for cgroups housekeeping, but I don't think it needs to actually be used for anything.
Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |