[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJniumW7ZKLLDaXSZ4TZ_=PX0ARERTeEyL-iTir12hciOArJqA@mail.gmail.com>
Date: Mon, 25 Jul 2011 18:17:08 -0700
From: Paul Menage <menage@...gle.com>
To: Frederic Weisbecker <fweisbec@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Li Zefan <lizf@...fujitsu.com>,
Johannes Weiner <hannes@...xchg.org>,
Aditya Kali <adityakali@...gle.com>
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@...il.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists