From: Oleg Nesterov <oleg@redhat.com>
To: Ben Blum <bblum@andrew.cmu.edu>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
fweisbec@gmail.com, neilb@suse.de, paul@paulmenage.org,
paulmck@linux.vnet.ibm.com
Subject: Re: + cgroups-more-safe-tasklist-locking-in-cgroup_attach_proc.patch added to -mm tree
Date: Fri, 2 Sep 2011 17:55:34 +0200 [thread overview]
Message-ID: <20110902155534.GA4595@redhat.com> (raw)
In-Reply-To: <20110902141550.GA24012@unix33.andrew.cmu.edu>
On 09/02, Ben Blum wrote:
>
> On Fri, Sep 02, 2011 at 04:00:15PM +0200, Oleg Nesterov wrote:
> > Forgot to mention, sorry...
> >
> > That said, I believe the patch is correct and should fix the problem.
>
> Thanks!
>
> But I don't think the check becomes pointless? If a sub-thread execs
> right before read_lock(&tasklist_lock) (but after the find_task_by_vpid
> in attach_task_by_pid), that causes the case that the comment refers to.
How so? The comment says:
* a race with de_thread from another thread's exec() may strip
* us of our leadership, making while_each_thread unsafe
This is not true.
And. Given that ->group_leader can be changed right after we drop tasklist
this check is pointless. Yes, it can detect the case when this task_struct
has nothing to do with this process sometimes, but not in general. (This
connects to other problems I mentioned).
IOW, personally I think it would be better to update the patch. But I
won't insist.
Oleg.
next prev parent reply other threads:[~2011-09-02 15:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-01 21:08 + cgroups-more-safe-tasklist-locking-in-cgroup_attach_proc.patch added to -mm tree akpm
2011-09-02 12:37 ` Oleg Nesterov
2011-09-02 14:00 ` Oleg Nesterov
2011-09-02 14:15 ` Ben Blum
2011-09-02 15:55 ` Oleg Nesterov [this message]
2011-09-07 23:59 ` Ben Blum
2011-09-08 17:35 ` Oleg Nesterov
2011-09-08 18:58 ` Ben Blum
2011-09-08 21:31 ` Oleg Nesterov
2011-09-09 2:11 ` Ben Blum
2011-09-09 16:41 ` Oleg Nesterov
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=20110902155534.GA4595@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bblum@andrew.cmu.edu \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=neilb@suse.de \
--cc=paul@paulmenage.org \
--cc=paulmck@linux.vnet.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.