lkml.org 
[lkml]   [2011]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [kernel.org users] [KORG] Panics on master backend
    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.

    > and thus its a
    > proper wakeup when we get here and thus ->pi_lock is serializing things.

    I am not sure. If ->on_rq is true, we need rq->lock. Say, pull_task() can
    change its cpu.

    > > --- x/kernel/sched.c
    > > +++ x/kernel/sched.c
    > > @@ -2694,10 +2694,11 @@ try_to_wake_up(struct task_struct *p, un
    > > goto out;
    > >
    > > success = 1; /* we're going to change ->state */
    > > - cpu = task_cpu(p);
    > >
    > > - if (p->on_rq && ttwu_remote(p, wake_flags))
    > > + if (p->on_rq && ttwu_remote(p, wake_flags)) {
    > > + cpu = task_cpu(p); /* for ttwu_stat() */
    > > goto stat;
    > > + }
    > >
    > > #ifdef CONFIG_SMP
    > > /*
    >
    > Would result in the same problem as below...

    I see, thanks.

    Yes, ttwu_queue(p, cpu) needs this task_cpu() without CONFIG_SMP.

    Oleg.



    \
     
     \ /
      Last update: 2011-08-25 16:01    [W:0.061 / U:0.012 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site