Re: [RFC PATCH 3/4] Container Freezer: Implement freezer cgroupsubsystem

From: Pavel Machek
Date: Fri Apr 11 2008 - 07:49:37 EST


Hi!

> This patch implements a new freezer subsystem for Paul Menage's
> control groups framework. It provides a way to stop and resume
> execution of all tasks in a cgroup by writing in the cgroup
> filesystem.
>
> This is the basic mechanism which should do the right thing for
> user space tasks in a simple scenario. This will require more work
> to get the freezing right (cf. try_to_freeze_tasks()) for ptraced
> tasks.

> --- /dev/null
> +++ linux-2.6.25-rc8-mm1/include/linux/cgroup_freezer.h
> @@ -0,0 +1,57 @@
> +#ifndef _LINUX_CGROUP_FREEZER_H
> +#define _LINUX_CGROUP_FREEZER_H
> +/*
> + * cgroup_freezer.h - control group freezer subsystem interface
> + *
> + * Copyright IBM Corp. 2007
> + *
> + * Author : Cedric Le Goater <clg@xxxxxxxxxx>
> + */

If you have copyright, add GPL.

> --- /dev/null
> +++ linux-2.6.25-rc8-mm1/kernel/cgroup_freezer.c
> @@ -0,0 +1,280 @@
> +/*
> + * cgroup_freezer.c - control group freezer subsystem
> + *
> + * Copyright IBM Corp. 2007
> + *
> + * Author : Cedric Le Goater <clg@xxxxxxxxxx>
> + */

Same here.

>+static struct cgroup_subsys_state *freezer_create(
>+ struct cgroup_subsys *ss, struct cgroup *cgroup)
>+{

Function headers are somehow non-traditional.

+ struct freezer *freezer;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return ERR_PTR(-EPERM);
+
+ freezer = kzalloc(sizeof(struct freezer), GFP_KERNEL);
+ if (!freezer)
+ return ERR_PTR(-ENOMEM);
+
+ spin_lock_init(&freezer->lock);
+ freezer->state = STATE_RUNNING;
+ return &freezer->css;
+}

One space too many after "return" :-).

Hmm, returning pointer inside struct freezer is rather ugly, right?
Could you just pass struct freezer around?
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/