public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serue@us.ibm.com>
To: Li Zefan <lizf@cn.fujitsu.com>
Cc: Linux Containers <containers@lists.linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Paul Menage <menage@google.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] devcgroup: avoid using cgroup_lock
Date: Fri, 13 Mar 2009 14:57:13 -0500	[thread overview]
Message-ID: <20090313195712.GA18439@us.ibm.com> (raw)
In-Reply-To: <20090313134139.GB32304@us.ibm.com>

Quoting Serge E. Hallyn (serue@us.ibm.com):
> Quoting Li Zefan (lizf@cn.fujitsu.com):
> > >> @@ -426,11 +431,11 @@ static int devcgroup_access_write(struct cgroup *cgrp, struct cftype *cft,
> > >>  				  const char *buffer)
> > >>  {
> > >>  	int retval;
> > >> -	if (!cgroup_lock_live_group(cgrp))
> > > 
> > > Does it matter that we no longer check for cgroup_is_removed()?
> > > 
> > 
> > No, this means in a rare case that the write handler is called when the cgroup
> > is dead, we still do the update work instead of returning ENODEV.
> > 
> > This is ok, since at that time, accessing cgroup and devcgroup is still valid,
> > but will have no effect since there is no task in this cgroup and the cgroup
> > will be destroyed soon.
> 
> Ok, just wanted to make sure the devcgroup couldn't be partially torn
> down and risking NULL or freed-memory derefs...

Ok, so the cgroup's files will be deleted first, then on the directory
removal the cgroup's data (each whitelist entry) is deleted.  So we can
let that ordering (by cgroup_clear_directory) ensure that nothing inside
a file write can happen while the destroy handler is called, right?

(That's why I was worried about not using the cgroup_lock: we need some
way of synchronizing those.  But I guess we're fine)

> BTW is that against linux-next?  (didn't seem to apply cleanly against
> my 2.6.29-rc9)  I guess I'd like to do a little test before acking,
> though it looks ok based on your answer.

Acked-by: Serge Hallyn <serue@us.ibm.com>

-serge

  reply	other threads:[~2009-03-13 19:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-12  1:47 [PATCH] devcgroup: avoid using cgroup_lock Li Zefan
2009-03-12 18:19 ` Serge E. Hallyn
2009-03-13  1:54   ` Li Zefan
2009-03-13 13:41     ` Serge E. Hallyn
2009-03-13 19:57       ` Serge E. Hallyn [this message]
2009-03-15  4:41         ` Li Zefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090313195712.GA18439@us.ibm.com \
    --to=serue@us.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=menage@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox