On Mon, Aug 31, 2015 at 10:32:50PM +0200, Peter Zijlstra wrote:
> On Mon, Aug 31, 2015 at 01:04:33PM -0500, Alex Thorlton wrote:
> q
> > diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> > index fd643d8..8502521 100644
> > --- a/kernel/stop_machine.c
> > +++ b/kernel/stop_machine.c
> > @@ -417,8 +417,11 @@ static void cpu_stopper_thread(unsigned int cpu)
> > {
> > struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu);
> > struct cpu_stop_work *work;
> > + unsigned long flags;
> > int ret;
> >
> > + local_irq_save(flags);
> > +
> > repeat:
> > work = NULL;
> > spin_lock_irq(&stopper->lock);
> > @@ -452,6 +455,8 @@ repeat:
> > cpu_stop_signal_done(done, true);
> > goto repeat;
> > }
> > +
> > + local_irq_restore(flags);
> > }
> >
>
> So I should probably just go sleep and not say anything.. _but_
> *confused*.
>
> That local_irq_save() will disable IRQs over:
>
> work = NULL;
>
> But that is _all_. The spin_unlock_irq() will re-enable IRQs, after
> which things will run as usual.
>
> That local_irq_restore() is a total NOP, IRQs are guaranteed enabled at
> the irq_local_save() (otherwise lockdep would've complained about
> spin_unlock_irq() unconditionally enabling them) and by the time we get
> to the restore that same unlock_irq will have enabled them already.
Ahh, right. Well that code is worthless then :)
Either way though, I guess that means that slight change just fudged the
timing enough in that area to avoid the lockup we're seeing. Ignoring
my useless code change, does anything else jump out at you as
interesting, here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
