Date: Mon, 07 Jan 2008 21:43:15 +0300 From: Alexey Starikovskiy <astarikovskiy@suse.de> To: "Moore, Robert" <robert.moore@intel.com> Cc: freebsd-acpi@FreeBSD.org Subject: Re: GPE handler livelock Message-ID: <478272C3.5080704@suse.de> In-Reply-To: <47827291.60405@suse.de> References: <200712311556.lBVFuVZf030567@freefall.freebsd.org><477916E0.2090702@root.org><200712311243.18123.jhb@freebsd.org> <47802510.3040203@root.org> <B2D86EF24E7E4BEF90BA00C945189463@kamino> <B28E9812BAF6E2498B7EC5C427F293A403F607B4@orsmsx415.amr.corp.intel.com> <47826AAA.6040101@root.org> <B28E9812BAF6E2498B7EC5C427F293A403F6087A@orsmsx415.amr.corp.intel.com> <47827291.60405@suse.de>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------080009060006020206020005 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Here is the patch... Alexey Starikovskiy wrote: > I proposed this patch some time ago, it moves _Lxx enabling to the end > of Notify queue, thus all notifies must complete before event becomes > enabled again. > Hope it is readable to non-Linux people... > > Regards, > Alex. > > Moore, Robert wrote: >> No changes that I know of before 20070508. >> >> You'll need to figure out why you are getting another GPE before the >> _Lxx method completes. There was something like this on Linux with an HP >> machine, perhaps Alexey can help. >> >> As I recall, there was something nasty happening where the TZ trip >> points had to be reset before the Notify() handler completed, but this >> ended up causing another GPE, etc. etc. >> >> Bob >> >> >>> -----Original Message----- >>> From: Nate Lawson [mailto:nate@root.org] >>> Sent: Monday, January 07, 2008 10:09 AM >>> To: Moore, Robert >>> Cc: Yousif Hassan; freebsd-acpi@FreeBSD.org >>> Subject: Re: GPE handler livelock >>> >>> Bob, thanks for the reply. That's exactly what my investigation is >>> showing also. It appears we're still on 20070320 so I'm not sure why >>> this would affect us though. Perhaps a similar change was already >>> present? In any case, we should see if an import fixes this. >>> >>> Thanks, >>> Nate >>> >>> Moore, Robert wrote: >>>> This sounds suspiciously like the changes we made to the Notify() >>>> handling last year. We attempted to make the notify handler run >>>> synchronously with the caller to Notify(), but this created more >>>> problems than it solved. We ended up returning the behavior of Notify >>>> handlers to be asynchronous: >>>> >>>> >>>> >>>> 19 October 2007. Summary of changes for version 20071019: >>>> >>>> 1) ACPI CA Core Subsystem: >>>> >>>> Reverted a change to Notify handling that was introduced in version >>>> 20070508. This version changed the Notify handling from asynchronous >> to >>>> fully synchronous (Device driver Notify handling with respect to the >>>> Notify >>>> ASL operator). It was found that this change caused more problems >> than >>>> it >>>> solved and was removed by most users. >>>> >>>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: owner-freebsd-acpi@freebsd.org [mailto:owner-freebsd- >>>>> acpi@freebsd.org] On Behalf Of Yousif Hassan >>>>> Sent: Sunday, January 06, 2008 12:18 PM >>>>> To: Nate Lawson >>>>> Cc: freebsd-acpi@FreeBSD.org >>>>> Subject: Re: GPE handler livelock >>>>> >>>>> Nate wrote: >>>>>> Thanks for digging into this. I reviewed this and am trying to >>>> figure >>>>>> out why the _L00 handler never completes. It keeps getting >> preempted >>>> by >>>>>> the next one. To help track this down, try removing these two >> lines >>>>>> from the _L00 method and recompile your ASL: >>>>>> >>>>>> Acquire (\_TZ.C173, 0xFFFF) >>>>>> ... >>>>>> Release (\_TZ.C173) >>>>>> >>>>>> For others who have this problem, instructions on how to recompile >>>> and >>>>>> load your custom ASL can be found here (11.16.4 and 5): >>>>>> >> http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/acpi-debug.htm >>>> l >>>>> I'm not sure if my situation is exactly what you're referring to in >>>>> this note, but since my laptop is also an HP (nx6110), and since it >>>> also >>>>> hangs during thermal zone changes, I tried your suggestion. It >> didn't >>>>> help, unfortunately. I still get mutex problems when the thermal >> zone >>>>> increases. My ASL did not have precisely the Acquire call you >> listed, >>>>> but it did have a similar one in method _L00 called >>>>> Acquire (\_TZ.C170, 0xFFFF) and Release (\_TZ.C170). Also this pair >>>>> of calls was also found in one other place (further down) in the >> ASL. >>>>> I only removed the first pair in the method you instructed. >>>>> FWIW, here are the debug error messages when the machine hangs: >>>>> >>>>> ACPI Exception (utmutex-0376): AE_TIME, Thread 28 could not acquire >>>> Mutex >>>>> [0] [20070320] >>>>> ACPI Error (exutils-0180): Could not acquire AML Interpreter mutex >>>>> [20070320] >>>>> ACPI Error (utmutex-0421): Mutex [0] is not acquired, cannot release >>>>> [20070320] >>>>> ACPI Error (exutils-0250): Could not release AML Interpreter mutex >>>>> [20070320] >>>>> ACPI Exception (utmutex-0376): AE_TIME, Thread 28 could not acquire >>>> Mutex >>>>> [0] [20070320] >>>>> ACPI Error (exutils-0180): Could not acquire AML Interpreter mutex >>>>> [20070320] >>>>> ACPI Error (psparse-0626): Method parse/execution failed >> [\_TZ_.C242] >>>> (Node >>>>> 0xc321c220), AE_TIME >>>>> ACPI Error (psparse-0626): Method parse/execution failed >>>> [\_TZ_.TZ1_._TMP] >>>>> (Node 0xc321b9c0), AE_TIME >>>>> ... etc ... >>>>> >>>>> I put more info into PR 79080. >>>>> >>>>> Let me know if you want me to try anything else. >>>>> --Yousif >>>>> >>>>> _______________________________________________ >>>>> freebsd-acpi@freebsd.org mailing list >>>>> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi >>>>> To unsubscribe, send any mail to >> "freebsd-acpi-unsubscribe@freebsd.org" >>> -- >>> Nate > > --------------080009060006020206020005 Content-Type: text/x-patch; name="defer_enabling_of_level_gpe.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="defer_enabling_of_level_gpe.patch" ACPI: Defer enabling of level GPE until all pending notifies done From: Alexey Starikovskiy <astarikovskiy@suse.de> Level GPE should not be enabled until all work caused by it is done, e.g. all Notify() methods are completed. This could be accomplished by appending enable_gpe function to the end of notify queue. Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de> --- drivers/acpi/events/evgpe.c | 17 +++++++++++++---- drivers/acpi/osl.c | 42 ++++++++---------------------------------- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c index e22f4a9..b4509f9 100644 --- a/drivers/acpi/events/evgpe.c +++ b/drivers/acpi/events/evgpe.c @@ -501,6 +501,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list) * an interrupt handler. * ******************************************************************************/ +static void acpi_ev_asynch_enable_gpe(void *context); static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context) { @@ -576,22 +577,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context) method_node))); } } + /* Defer enabling of GPE until all notify handlers are done */ + acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_asynch_enable_gpe, + gpe_event_info); + return_VOID; +} - if ((local_gpe_event_info.flags & ACPI_GPE_XRUPT_TYPE_MASK) == +static void acpi_ev_asynch_enable_gpe(void *context) +{ + struct acpi_gpe_event_info *gpe_event_info = context; + acpi_status status; + if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) == ACPI_GPE_LEVEL_TRIGGERED) { /* * GPE is level-triggered, we clear the GPE status bit after * handling the event. */ - status = acpi_hw_clear_gpe(&local_gpe_event_info); + status = acpi_hw_clear_gpe(gpe_event_info); if (ACPI_FAILURE(status)) { return_VOID; } } /* Enable this GPE */ - - (void)acpi_hw_write_gpe_enable_reg(&local_gpe_event_info); + (void)acpi_hw_write_gpe_enable_reg(gpe_event_info); return_VOID; } diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index aabc6ca..6816ac6 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -625,25 +625,6 @@ static void acpi_os_execute_deferred(struct work_struct *work) dpc->function(dpc->context); kfree(dpc); - /* Yield cpu to notify thread */ - cond_resched(); - - return; -} - -static void acpi_os_execute_notify(struct work_struct *work) -{ - struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work); - - if (!dpc) { - printk(KERN_ERR PREFIX "Invalid (NULL) context\n"); - return; - } - - dpc->function(dpc->context); - - kfree(dpc); - return; } @@ -667,7 +648,7 @@ acpi_status acpi_os_execute(acpi_execute_type type, { acpi_status status = AE_OK; struct acpi_os_dpc *dpc; - + struct workqueue_struct *queue; ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "Scheduling function [%p(%p)] for deferred execution.\n", function, context)); @@ -691,20 +672,13 @@ acpi_status acpi_os_execute(acpi_execute_type type, dpc->function = function; dpc->context = context; - if (type == OSL_NOTIFY_HANDLER) { - INIT_WORK(&dpc->work, acpi_os_execute_notify); - if (!queue_work(kacpi_notify_wq, &dpc->work)) { - status = AE_ERROR; - kfree(dpc); - } - } else { - INIT_WORK(&dpc->work, acpi_os_execute_deferred); - if (!queue_work(kacpid_wq, &dpc->work)) { - ACPI_DEBUG_PRINT((ACPI_DB_ERROR, - "Call to queue_work() failed.\n")); - status = AE_ERROR; - kfree(dpc); - } + INIT_WORK(&dpc->work, acpi_os_execute_deferred); + queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq; + if (!queue_work(queue, &dpc->work)) { + ACPI_DEBUG_PRINT((ACPI_DB_ERROR, + "Call to queue_work() failed.\n")); + status = AE_ERROR; + kfree(dpc); } return_ACPI_STATUS(status); } --------------080009060006020206020005--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?478272C3.5080704>