Messages in this thread | | | Date | Thu, 16 Jan 2014 13:14:15 +0100 | From | Peter Zijlstra <> | Subject | Re: linux-next: build failure after merge of the tip tree |
| |
On Tue, Jan 14, 2014 at 04:43:43PM -0500, Mikulas Patocka wrote: > > @@ -200,7 +201,9 @@ static void speedstep_set_state(unsigned int state) > > if (retry) { > > pr_debug("retry %u, previous result %u, waiting...\n", > > retry, result); > > + local_irq_restore(flags); > > ^^^ this is wrong, because the function speedstep_set_state may already be > called with interrupts disabled from speedstep_get_freqs. So, you need to > enable interrupts unconditionally, even if they were disabled at the > beginning of the function speedstep_set_state. > > I know it's dirty to enable interrupts in a function that was called with > disabled interrupts, but here it must be so (you could rewrite > speedstep_get_freqs to not disable interrupts if you want to avoid this > dirtiness).
Egads; I think you had better, this is vile beyond reason.
> > mdelay(retry * 50); > > + local_irq_save(flags); > > } > > retry++; > > __asm__ __volatile__( > > @@ -217,6 +220,7 @@ static void speedstep_set_state(unsigned int state) > > > > /* enable IRQs */ > > local_irq_restore(flags); > > + preempt_enable(); > > > > if (new_state == state) > > pr_debug("change to %u MHz succeeded after %u tries " > > You need also preempt_disable/enable in speedstep_get_freqs.
Argh I see, this is really horrid.
Anyway, its Rafael's call, its his subsystem he gets to fix it when it explodes.
/me shudders
| |