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.
next 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.