All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Stefan Bader <stefan.bader@canonical.com>,
	mingo@kernel.org, Oleg Nesterov <oleg@redhat.com>,
	Paul Turner <pjt@google.com>, Mike Galbraith <efault@gmx.de>,
	Andrew Vagin <avagin@openvz.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [RFC][PATCH] sched: Fix race in task_group()
Date: Tue, 26 Jun 2012 13:13:39 -0700	[thread overview]
Message-ID: <20120626201339.GU3869@google.com> (raw)
In-Reply-To: <1340718515.21991.83.camel@twins>

On Tue, Jun 26, 2012 at 03:48:35PM +0200, Peter Zijlstra wrote:
> Here's one that's actually compile tested (with the right CONFIG_foo
> enabled) and I fixed the autogroup lockdep splat.
> 
> ---
> Subject: sched: Fix race in task_group()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Fri, 22 Jun 2012 13:36:05 +0200
> 
> Stefan reported a crash on a kernel before a3e5d1091c1 ("sched: Don't
> call task_group() too many times in set_task_rq()"), he found the reason
> to be that the multiple task_group() invocations in set_task_rq()
> returned different values.

Hmm... short of intertwining locking further I don't think we can
solve this in prettier way.  So, yeah, looks good to me from cgroup
POV.

> Looking at all that I found a lack of serialization and plain wrong
> comments.
> 
> The below tries to fix it using an extra pointer which is updated under
> the appropriate scheduler locks. Its not pretty, but I can't really see
> another way given how all the cgroup stuff works.

BTW your patch is whitespace broken.  Seems like QP encoded.

Thanks.

-- 
tejun

  parent reply	other threads:[~2012-06-26 20:13 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-22 11:36 [RFC][PATCH] sched: Fix race in task_group() Peter Zijlstra
2012-06-22 15:06 ` Stefan Bader
2012-06-22 15:15   ` Peter Zijlstra
2012-06-26 13:48   ` Peter Zijlstra
2012-06-26 17:49     ` Stefan Bader
2012-06-27 12:40       ` Hillf Danton
2012-06-27 12:51         ` Stefan Bader
2012-06-26 20:13     ` Tejun Heo [this message]
2012-06-26 21:17       ` Peter Zijlstra
2012-07-03 10:06     ` Stefan Bader
2012-07-06  6:24 ` [tip:sched/core] " tip-bot for Peter Zijlstra
2012-07-24 14:21 ` tip-bot for Peter Zijlstra
2012-10-18  8:27   ` cwillu
2012-10-18 10:23     ` Stefan Bader
2012-10-18 13:33       ` Luis Henriques
2012-10-18 20:50         ` cwillu
2012-10-19  7:40         ` Stefan Bader

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=20120626201339.GU3869@google.com \
    --to=tj@kernel.org \
    --cc=avagin@openvz.org \
    --cc=efault@gmx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=stefan.bader@canonical.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.