Re: [PATCH -mmotm 1/8] cgroup: introduce cancel_attach()

Daisuke Nishimura
2009-11-09T07:43:24Z

On Mon, 09 Nov 2009 15:23:02 +0800, Li Zefan <lizf@cn.fujitsu.com> wrote:
> Daisuke Nishimura wrote:
> > This patch adds cancel_attach() operation to struct cgroup_subsys.
> > cancel_attach() can be used when can_attach() operation prepares something
> > for the subsys, but we should rollback what can_attach() operation has prepared
> > if attach task fails after we've succeeded in can_attach().
> >
> > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
>
> Acked-by: Li Zefan <lizf@cn.fujitsu.com>
>
Thank you for your ack.

> > ---
> > Documentation/cgroups/cgroups.txt | 13 +++++++++++-
> > include/linux/cgroup.h | 2 +
> > kernel/cgroup.c | 38 ++++++++++++++++++++++++++++++------
> > 3 files changed, 45 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/cgroups/cgroups.txt b/Documentation/cgroups/cgroups.txt
> > index 0b33bfe..c86947c 100644
> > --- a/Documentation/cgroups/cgroups.txt
> > +++ b/Documentation/cgroups/cgroups.txt
> > @@ -536,10 +536,21 @@ returns an error, this will abort the attach operation. If a NULL
> > task is passed, then a successful result indicates that *any*
> > unspecified task can be moved into the cgroup. Note that this isn't
> > called on a fork. If this method returns 0 (success) then this should
> > -remain valid while the caller holds cgroup_mutex. If threadgroup is
> > +remain valid while the caller holds cgroup_mutex and it is ensured that either
> > +attach() or cancel_attach() will be called in futer. If threadgroup is
>
> s/futer/future
>
Ah, I'm sorry.
will fix.

> > true, then a successful result indicates that all threads in the given
> > thread's threadgroup can be moved together.
> ...
> > +out:
> > + if (retval)
>
> I prefer:
>
> if (reval) {
> ...
> }
>
Okey.
will change in next post.

Thanks,
Daisuke Nishimura.

> > + for_each_subsys(root, ss) {
> > + if (ss == failed_ss)
> > + /*
> > + * This means can_attach() of this subsystem
> > + * have failed, so we don't need to call
> > + * cancel_attach() against rests of subsystems.
> > + */
> > + break;
> > + if (ss->cancel_attach)
> > + ss->cancel_attach(ss, cgrp, tsk, false);
> > + }
> > + return retval;
> > }
> >
> > /*