Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 07 Jan 2008 15:47:48 -0800
From:      Nate Lawson <nate@root.org>
To:        Alexey Starikovskiy <astarikovskiy@suse.de>
Cc:        freebsd-acpi@FreeBSD.org, "Moore, Robert" <robert.moore@intel.com>
Subject:   Re: GPE handler livelock
Message-ID:  <4782BA24.8030703@root.org>
In-Reply-To: <4782A051.3050107@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> <478272C3.5080704@suse.de> <47829C03.3010802@root.org> <4782A051.3050107@suse.de>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070305010103020303040108
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

This makes sense.  For the _L00 method in question, it runs 2 notifies
before it completes.  So in our queue, that would be:

T1: GPE:_L00
    [_LOO method runs, GPE removed]
    [_L00 queues 2 more Notifies]
T2: Notify, Notify
    [_L00 completes]

Right now, there is nothing in our code to synchronize _L00's completion
with completion of the Notifys.  In fact, _L00 finishes before the
Notifys run.  Also, if another GPE comes in while this GPE is running,
it will be queued in front of the Notifys and so would execute first.

So I think with just your evgpe.c change, we will defer re-enabling the
GPE until after the queued Notifys complete since the re-enable task
will be queued at the same priority.

The only remaining concern I have is if another GPE comes in before one
or both Notifys run, it will be inserted at the head of the queue.  So
as a hack, I'm setting the priority of these to be equal.

Yousif, please try the attached patch and don't load a custom ASL.

Thanks,
Nate

Alexey Starikovskiy wrote:
> Nate,
> 
> There are no debugger events in Linux, and all other deferred execution
> happens
> in kacpid (GPE, EC and GL included). Notify events used to be executed
> on this same
> queue until we noticed deadlocks with HP machines. All events are not
> prioritized in
> any way -- it is a simple FIFO. To avoid deadlock, we moved notify
> events to separate queue,
> but it had a drawback of enabling level GPE too early, thus I inserted a
> reschedule call to each completion on first queue, giving the notify
> queue chance to complete.
> Later, I was reminded that this approach is not bulletproof, so enabling
> of the level events was
> moved to notify queue as well. As it happens after all notify events for
> the gpe event were called,
> (but, probably, not executed), enabling of GPE will be deferred until
> these notify events have chance to
> complete.
> 
> So, essentially, we had no priority for any event, but now notify event
> could preempt execution of
> any other event, and level GPE event does a flush of notify queue.
> 
> Hope this helps,
> Alex.
> 
> Nate Lawson wrote:
>> Alex,
>>
>> I had one question about your approach.  It maintains two single-thread
>> task queues (kacpid and kacpi_notify).  It inserts each type of event on
>> its own queue.  So there is no strict ordering of handling notifies in
>> priority to other acpi tasks unless you're assuming something about the
>> linux task priority model.  Do you have any expectation that notify
>> tasks run before other tasks (perhaps by a special priority assigned to
>> the kacpi_notify work queue)?
>>
>> In FreeBSD, we have a single task queue.  However, we prioritize events
>> in the queue in the following order (highest to lowest priority):
>>
>> * GPE
>> * EC/global lock
>> * Notify
>> * Debugger
>>
>> If an event is inserted on the queue with a higher priority and a
>> previous event has not started executing yet, this priority determines
>> the order of insertion.  Thus if GPEs keep arriving, the Notify won't be
>> executed until they're done.
>>
>> Thanks,
>> Nate
>>
>> Alexey Starikovskiy wrote:
>>> 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
>>


--------------070305010103020303040108
Content-Type: text/x-patch;
 name="defer_gpe1.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="defer_gpe1.diff"

Index: sys/dev/acpica/Osd/OsdSchedule.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/acpica/Osd/OsdSchedule.c,v
retrieving revision 1.39
diff -u -r1.39 OsdSchedule.c
--- sys/dev/acpica/Osd/OsdSchedule.c	22 Mar 2007 18:16:41 -0000	1.39
+++ sys/dev/acpica/Osd/OsdSchedule.c	7 Jan 2008 23:46:47 -0000
@@ -114,7 +114,7 @@
 	pri = 5;
 	break;
     case OSL_NOTIFY_HANDLER:
-	pri = 3;
+	pri = 10; // XXX testing
 	break;
     case OSL_DEBUGGER_THREAD:
 	pri = 0;
Index: sys/contrib/dev/acpica/evgpe.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/dev/acpica/evgpe.c,v
retrieving revision 1.1.1.12
diff -u -r1.1.1.12 evgpe.c
--- sys/contrib/dev/acpica/evgpe.c	22 Mar 2007 17:23:30 -0000	1.1.1.12
+++ sys/contrib/dev/acpica/evgpe.c	7 Jan 2008 21:17:28 -0000
@@ -123,6 +123,10 @@
 
 /* Local prototypes */
 
+static void
+AcpiEvAsynchEnableGpe (
+    void                    *Context);
+
 static void ACPI_SYSTEM_XFACE
 AcpiEvAsynchExecuteGpeMethod (
     void                    *Context);
@@ -684,14 +688,26 @@
         }
     }
 
-    if ((LocalGpeEventInfo.Flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
+    /* Defer enabling of GPE until all notify handlers are done */
+    AcpiOsExecute(OSL_NOTIFY_HANDLER, AcpiEvAsynchEnableGpe, GpeEventInfo);
+    return_VOID;
+}
+
+static void
+AcpiEvAsynchEnableGpe (
+    void                    *Context)
+{
+    ACPI_GPE_EVENT_INFO     *GpeEventInfo = (void *) Context;
+    ACPI_STATUS             Status;
+
+    if ((GpeEventInfo->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 = AcpiHwClearGpe (&LocalGpeEventInfo);
+        Status = AcpiHwClearGpe (GpeEventInfo);
         if (ACPI_FAILURE (Status))
         {
             return_VOID;
@@ -700,7 +716,7 @@
 
     /* Enable this GPE */
 
-    (void) AcpiHwWriteGpeEnableReg (&LocalGpeEventInfo);
+    (void) AcpiHwWriteGpeEnableReg (GpeEventInfo);
     return_VOID;
 }
 

--------------070305010103020303040108--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4782BA24.8030703>