All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@elte.hu>
Cc: Ben Blum <bblum@google.com>, Jiri Slaby <jirislaby@gmail.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	Li Zefan <lizf@cn.fujitsu.com>, Miao Xie <miaox@cn.fujitsu.com>,
	Paul Menage <menage@google.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 0/6] sched/cpusets fixes, more changes are needed
Date: Mon, 15 Mar 2010 10:09:58 +0100	[thread overview]
Message-ID: <20100315090958.GA9116@redhat.com> (raw)

Ingo, Peter.

Unless I missed something, with or without these patches the TASK_WAKING
logic in do_fork() is very broken.

	- do_fork() clears PF_STARTING and then calls wake_up_new_task()
	  which finally does s/WAKING/RUNNING.

	  But. Nobody can take rq->lock in between. This means a signal
	  from irq (quite possible with CLONE_THREAD) or another rt
	  thread which preempts us can lockup.

	- the comment in wake_up_new_task says:
	
		We still have TASK_WAKING but PF_STARTING is gone now, meaning
		->cpus_allowed is stable

	  this is not true. Yes, nobody can take rq->lock _after_ we cleared
	  PF_STARTING, but it is possible that another thread took this lock
	  before and still holds it doing, say, sched_setaffinity().

No?

If yes. I can make a patch, but the question is: what is the point to use
TASK_WAKING in fork pathes? Can't sched_fork() set TASK_RUNNING instead?
Afaics, TASK_RUNNING can equally protect from premature wakeups but doesn't
these PF_STARTING complications.

As for this series. Please review. I don't understand how it is possible
to really test these changes.

Dear cpuset developers! Please review ;) If you don't like 6/6, please make
a better fix. I tried to make as "simple" patch as possible because I hardly
understand cpuset.c, last time I quickly read it a long ago.

Oleg.


             reply	other threads:[~2010-03-15  9:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-15  9:09 Oleg Nesterov [this message]
2010-03-24 17:38 ` [PATCH 0/6] sched/cpusets fixes, more changes are needed Peter Zijlstra
2010-03-24 18:09   ` Oleg Nesterov
2010-03-25 10:22     ` Peter Zijlstra
2010-03-25 15:46       ` Oleg Nesterov
2010-03-25 16:02         ` Oleg Nesterov
2010-03-25 16:10           ` Oleg Nesterov
2010-03-25 17:29             ` Peter Zijlstra
2010-03-25 19:15               ` 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=20100315090958.GA9116@redhat.com \
    --to=oleg@redhat.com \
    --cc=bblum@google.com \
    --cc=jirislaby@gmail.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=menage@google.com \
    --cc=miaox@cn.fujitsu.com \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=rjw@sisk.pl \
    --cc=tj@kernel.org \
    /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.