From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH] cpuidle: delay enabling interrupts until all coupled CPUs leave idle Date: Thu, 06 Mar 2014 13:55:06 +0100 Message-ID: <6009429.TU4uH7j3ue@vostro.rjw.lan> References: <1394103721-32616-1-git-send-email-paul.burton@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from v094114.home.net.pl ([79.96.170.134]:58843 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750760AbaCFMjy (ORCPT ); Thu, 6 Mar 2014 07:39:54 -0500 In-Reply-To: <1394103721-32616-1-git-send-email-paul.burton@imgtec.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Paul Burton Cc: linux-pm@vger.kernel.org, Daniel Lezcano On Thursday, March 06, 2014 11:02:01 AM Paul Burton wrote: > As described by a comment at the end of cpuidle_enter_state_coupled it > can be inefficient for coupled idle states to return with IRQs enabled > since they may proceed to service an interrupt instead of clearing the > coupled idle state. Until they have finished & cleared the idle state > all CPUs coupled with them will spin rather than being able to enter a > safe idle state. > > Commits e1689795a784 "cpuidle: Add common time keeping and irq > enabling" and 554c06ba3ee2 "cpuidle: remove en_core_tk_irqen flag" led > to the cpuidle_enter_state enabling interrupts for all idle states, > including coupled ones, making this inefficiency unavoidable by drivers > & the local_irq_enable near the end of cpuidle_enter_state_coupled > redundant. This patch avoids enabling interrupts in cpuidle_enter_state > after a coupled state has been entered, allowing them to remain disabled > until all coupled CPUs have exited the idle state and > cpuidle_enter_state_coupled re-enables them. This appears to be a regression. Are there any bug reports related to it? Alternatively, can you reproduce it and if so, then on what hardware? > Signed-off-by: Paul Burton > Cc: Rafael J. Wysocki > Cc: Daniel Lezcano > Cc: linux-pm@vger.kernel.org > --- > drivers/cpuidle/cpuidle.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > index a55e68f..366e684 100644 > --- a/drivers/cpuidle/cpuidle.c > +++ b/drivers/cpuidle/cpuidle.c > @@ -85,7 +85,8 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, > > time_end = ktime_get(); > > - local_irq_enable(); > + if (!cpuidle_state_is_coupled(dev, drv, entered_state)) > + local_irq_enable(); > > diff = ktime_to_us(ktime_sub(time_end, time_start)); > if (diff > INT_MAX) > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.