From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: linux-next: build failure after merge of the tip tree Date: Thu, 16 Jan 2014 13:14:15 +0100 Message-ID: <20140116121415.GP31570@twins.programming.kicks-ass.net> References: <20140114142627.537a702607e703c6eff63640@canb.auug.org.au> <20140114141406.GD7572@laptop.programming.kicks-ass.net> <20140114200547.GM7572@laptop.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: cpufreq-owner@vger.kernel.org To: Mikulas Patocka Cc: Stephen Rothwell , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "Rafael J. Wysocki" , Viresh Kumar , linux-next@vger.kernel.org, linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org List-Id: linux-next.vger.kernel.org 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