linux-kernel - Re: [PATCH 7/7] cgroups: Add a max number of tasks subsystem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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/