From mboxrd@z Thu Jan 1 00:00:00 1970 From: preeti Subject: Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver Date: Mon, 02 Jul 2012 10:07:35 +0530 Message-ID: <4FF1258F.7020805@linux.vnet.ibm.com> References: <4FEC20D2.2070501@linux.vnet.ibm.com> <4FED2AF2.4080900@linux.vnet.ibm.com> <4FED3CA0.3050909@linux.vnet.ibm.com> <201206300011.51402.rjw@sisk.pl> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: Received: from e23smtp06.au.ibm.com ([202.81.31.148]:35646 "EHLO e23smtp06.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751247Ab2GBEc3 (ORCPT ); Mon, 2 Jul 2012 00:32:29 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 2 Jul 2012 04:20:00 +1000 In-Reply-To: <201206300011.51402.rjw@sisk.pl> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: "Rafael J. Wysocki" Cc: Dave Hansen , Linux PM mailing list , linux-pm@lists.linux-foundation.org, linux-acpi@vger.kernel.org, "Srivatsa.S.Bhat" , Deepthi Dharwar On 06/30/2012 03:41 AM, Rafael J. Wysocki wrote: > On Friday, June 29, 2012, preeti wrote: >> On 06/29/2012 09:41 AM, preeti wrote: >>> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote: >>>> On Thursday, June 28, 2012, preeti wrote: >>>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote: >>>>>> On Thursday, June 28, 2012, preeti wrote: >>>>>>> >>>>>>> From: Preeti U Murthy >>>> [...] >>>>> cpuidle is an architecture independent part of the kernel code.Since >>>>> this patch aims at x86 architecture in specific,I considered it >>>>> inappropriate. >>>>> >>>>> In addition to this,suspend happens on x86 only if ACPI is configured. >>>> >>>> But that is not required for intel_idle, so if it hangs with intel_idle, >>>> then it is not dependent on ACPI after all. >>> True intel_idle does not need ACPI to be configured,but that also means >>> that the kernel will not provide you the means to suspend.There is no >>> question of resume hang here at all as you cannot suspend in the first >>> place. >>> >>> The issue is when ACPI is configured,and intel_idle is chosen to be the >>> cpuidle driver.In this situation when the user suspends,cpus can enter >>> deep sleep states as intel_idle driver does not prevent then from doing so. >>> This is when resume hangs. >>>> >>>>> Therefore it seemed right to put the callback in ACPI specific code >>>>> which deals with ACPI sleep support. >>>> >>>> I wonder if we can address this issue correctly. That is, in a non-racy >>>> way and in a better place. >>>> >>>> First, I really don't think it is necessary to "suspend" cpuidle (be it >>>> ACPI or any other) when device drivers' suspend routines are being >>>> executed (which also is racy, because the cpuidle "suspend" may be running >>>> concurrently with cpuidle on another CPU) or earlier. We really may want >>>> to disable the deeper C-states when we're about to execute >>>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after >>>> we've run dpm_suspend_end() successfully. >>> >>> The commit "ACPI:disable lower idle C-states across suspend/resume" >>> states that device_suspend() calls ACPI suspend functions which cause >>> side effects on the lower idle C-states.This means we need to disable >>> entry into deeper C-states even before dpm_suspend_start(),but how much >>> before? >>> >>> The commit answers this too.It says removing the functionality of >>> entering deep C-states before suspend removed the side effects.Besides, >>> the commit title says'across suspend/resume'.So I think to address the >>> resume hang effectively,it is desirable to disable entry into deeper >>> C-states during suspend_prepare operations. >> >> To clarify this further,since we take action upon PM_SUSPEND_PREPARE >> notification,which is called before suspend begins,we avoid race >> condition between suspend operations and disabling entry into deeper >> c-states altogether. > > Well, what about races between disabling deeper C-states and cpuidle? Yes.The question still remains about the cpus that have already entered deep C-states even before suspend routines have begun.We are not taking precautions to prevent them from going into idle. If the resume hang does depend on the cpus being in deep C-state,even after the fix with acpi_idle_suspend, there should have been a hang in scenarios where the cpus have already entered deep C-states before suspend has begun. > > Rafael > Regards Preeti