lkml.org 
[lkml]   [2011]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [kernel.org users] [KORG] Panics on master backend
On Thu, Aug 25, 2011 at 03:54:29PM +0200, Oleg Nesterov wrote:
> On 08/25, Peter Zijlstra wrote:
> >
> > On Wed, 2011-08-24 at 18:08 +0200, Oleg Nesterov wrote:
> > > >
> > > > static void ttwu_queue(struct task_struct *p, int cpu)
> > > > @@ -2705,7 +2703,6 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> > > > * this task as prev, wait until its done referencing the task.
> > > > */
> > > > while (p->on_cpu) {
> > > > -#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
> > > > /*
> > > > * In case the architecture enables interrupts in
> > > > * context_switch(), we cannot busy wait, since that
> > > > @@ -2713,11 +2710,11 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> > > > * tries to wake up @prev. So bail and do a complete
> > > > * remote wakeup.
> > > > */
> > > > - if (ttwu_activate_remote(p, wake_flags))
> > > > + if (cpu == smp_processor_id() &&
> > >
> > > I think this needs "task_cpu(p) == smp_processor_id()". We can't trust
> > > "cpu", task_cpu() was called before ->on_rq check.
> >
> > Isn't us holding ->pi_lock sufficient to stabilize task_cpu()? If its a
> > running task the initial ->state check would have failed,
>
> Of course it is not TASK_RUNNING, but it can be running or not.

Yup. Before we go beyond ttwu_remote() in ttwu(), 'cpu' is not safe.
For example, wait_event() could be preempted in between.

But after we go beyond ttwu_remote(), ->pi_lock will stabilize it.

So after we take Oleg's suggestion("task_cpu(p) == smp_processor_id()"),
things we left is just how to account stat correctly.

IMHO, we could get cpu in ttwu_remote() to prevent the side effect of
pull_task().

something like below?

Thanks,
Yong

---
diff --git a/kernel/sched.c b/kernel/sched.c
index ccacdbd..4a1d05d 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2540,7 +2540,7 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags)
* since all we need to do is flip p->state to TASK_RUNNING, since
* the task is still ->on_rq.
*/
-static int ttwu_remote(struct task_struct *p, int wake_flags)
+static int ttwu_remote(struct task_struct *p, int wake_flags, int *cpu)
{
struct rq *rq;
int ret = 0;
@@ -2548,6 +2548,7 @@ static int ttwu_remote(struct task_struct *p, int wake_flags)
rq = __task_rq_lock(p);
if (p->on_rq) {
ttwu_do_wakeup(rq, p, wake_flags);
+ *cpu = task_cpu(p);
ret = 1;
}
__task_rq_unlock(rq);
@@ -2696,7 +2697,12 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
success = 1; /* we're going to change ->state */
cpu = task_cpu(p);

- if (p->on_rq && ttwu_remote(p, wake_flags))
+ /*
+ * read cpu for another time if ttwu_remote() success,
+ * just to prevent task migration in between, otherwise
+ * we maybe account stat incorrectly.
+ */
+ if (p->on_rq && ttwu_remote(p, wake_flags, &cpu))
goto stat;

#ifdef CONFIG_SMP


\
 
 \ /
  Last update: 2011-08-26 08:03    [W:0.580 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site