Re: [RFC PATCH] cgroup: add cgroup.signal

["Michal Koutný" <mkoutny-IBi9RG/b67k@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Fri, Apr 23, 2021 at 07:13:51PM +0200, Christian Brauner <brauner-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> +static void __cgroup_signal(struct cgroup *cgrp, int signr)
> +{
> +	struct css_task_iter it;
> +	struct task_struct *task;
> +
> +	lockdep_assert_held(&cgroup_mutex);
> +
> +	css_task_iter_start(&cgrp->self, 0, &it);

I think here you'd need CSS_TASK_ITER_PROCS here to avoid signalling
multithreaded processes multiple times? (OTOH, with commiting just to
SIGKILL this may be irrelevant.)

> +static void cgroup_signal(struct cgroup *cgrp, int signr)
> [...]
> +	read_lock(&tasklist_lock);

(Thinking loudly.)
I wonder if it's possible to get rid of this? A similar check that
freezer does in cgroup_post_fork() but perhaps in cgroup_can_fork(). The
fork/clone would apparently fail for the soon-to-die parent but there's
already similar error path returning ENODEV (heh, the macabrous name
cgroup_is_dead() is already occupied).

This way the cgroup-killer wouldn't unncessarily preempt tasklist_lock
exclusive requestors system-wide.


> @@ -4846,6 +4916,11 @@ static struct cftype cgroup_base_files[] = {
> +	{
> +		.name = "cgroup.signal",
> +		.flags = CFTYPE_NOT_ON_ROOT,
> +		.write = cgroup_signal_write,

I think this shouldn't be visible in threaded cgroups (or return an
error when attempting to kill those).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]