Re: ACPI panic
On Mon, Nov 26, 2012 at 01:13:46PM +0200, Andriy Gapon wrote:
Also, I've just realized that the check is racy...
Could you please move the whole check block (between and excluding
AcpiUtAcquireMutex and AcpiUtReleaseMutex) down right below the following
lines:
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
return (Status);
}
Sorry for the delay. I'm now running the patch below. I still got the
cycle panic, this time with a 4-objects cycle. It looks like an object
gets released twice but I don't understand why the freeing a free
object check fails to trigger.
Stefan
Index: components/utilities/utcache.c
===
--- components/utilities/utcache.c (revision 243234)
+++ components/utilities/utcache.c (working copy)
@@ -244,6 +244,28 @@
return (Status);
}
+char*Curr;
+char*Next;
+int Depth;
+Depth = Cache-CurrentDepth;
+Next = Cache-ListHead;
+while (Next)
+{
+Curr = Next;
+Next = *(ACPI_CAST_INDIRECT_PTR (char,
+(((char *) Curr)[Cache-LinkOffset])));
+if (*(const unsigned char *) Curr != 0xCA) {
+panic(detected use after free %p\n, Curr);
+}
+if (Object == Curr) {
+panic(freeing a free object %p, Object);
+}
+Depth--;
+if (Depth 0) {
+panic(cycle in a cache list);
+}
+}
+
/* Mark the object as cached */
ACPI_MEMSET (Object, 0xCA, Cache-ObjectSize);
@@ -312,6 +334,10 @@
Cache-CurrentDepth--;
+if (*(const unsigned char *) Object != 0xCA) {
+panic(detected use after free %p\n, Object);
+}
+
ACPI_MEM_TRACKING (Cache-Hits++);
ACPI_DEBUG_PRINT ((ACPI_DB_EXEC,
Object %p from %s cache\n, Object, Cache-ListName));
___
freebsd-acpi@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: [rfc] bind curthread to target cpu for _CST change notification
Turned out not be so rosy... on 25/11/2012 21:37 Sean Bruno said the following: On Thu, 2012-11-22 at 16:53 +0200, Andriy Gapon wrote: I would like to propose the following patch which should kill two birds with one stone: - avoid race in acpi_cpu_cx_cst if more than one notifications occur in a rapid succession for the same CPU and end up being concurrently handled by ACPI taskqeue threads critical_enter was a very a bad idea and can't be used here because acpi_cpu_cx_cst acquires blockable locks and does waiting memory allocations. Unfortunately, it was not immediately obvious to me. - avoid race acpi_cpu_cx_cst and acpi_cpu_idle where the latter may access resources being updated by the former sched_bind wouldn't guarantee that this would work if critical_enter is not used, because the current thread may block and the idle thread may get to run. What do you think? Index: sys/dev/acpica/acpi_cpu.c === --- sys/dev/acpica/acpi_cpu.c(revision 242854) +++ sys/dev/acpica/acpi_cpu.c(working copy) @@ -1047,7 +1047,16 @@ return; /* Update the list of Cx states. */ +thread_lock(curthread); +sched_bind(curthread, sc-cpu_pcpu-pc_cpuid); +thread_unlock(curthread); +critical_enter(); acpi_cpu_cx_cst(sc); +critical_exit(); +thread_lock(curthread); +sched_unbind(curthread); +thread_unlock(curthread); + acpi_cpu_cx_list(sc); ACPI_SERIAL_BEGIN(cpu); Ack. This looks appropriate to me. I am working on an alternative approach to these two issues. Thank you. -- Andriy Gapon ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ACPI panic
on 29/11/2012 10:46 Stefan Farfeleder said the following:
On Mon, Nov 26, 2012 at 01:13:46PM +0200, Andriy Gapon wrote:
Also, I've just realized that the check is racy...
Could you please move the whole check block (between and excluding
AcpiUtAcquireMutex and AcpiUtReleaseMutex) down right below the following
lines:
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
return (Status);
}
Sorry for the delay. I'm now running the patch below. I still got the
cycle panic, this time with a 4-objects cycle. It looks like an object
gets released twice but I don't understand why the freeing a free
object check fails to trigger.
Hmmm...
Another bug-catching patch before I start questioning my ability to understand
the code.
index 59ecf21..1687c75b 100644
--- a/sys/contrib/dev/acpica/components/utilities/utcache.c
+++ b/sys/contrib/dev/acpica/components/utilities/utcache.c
@@ -238,6 +238,8 @@ AcpiOsReleaseObject (
else
{
+if (AcpiGbl_MutexInfo[ACPI_MTX_CACHES].ThreadId == AcpiOsGetThreadId
())
+panic(ACPI_MTX_CACHES acquired recursively);
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
@@ -311,6 +313,8 @@ AcpiOsAcquireObject (
return (NULL);
}
+if (AcpiGbl_MutexInfo[ACPI_MTX_CACHES].ThreadId == AcpiOsGetThreadId ())
+panic(ACPI_MTX_CACHES acquired recursively);
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
Stefan
Index: components/utilities/utcache.c
===
--- components/utilities/utcache.c(revision 243234)
+++ components/utilities/utcache.c(working copy)
@@ -244,6 +244,28 @@
return (Status);
}
+char*Curr;
+char*Next;
+int Depth;
+Depth = Cache-CurrentDepth;
+Next = Cache-ListHead;
+while (Next)
+{
+Curr = Next;
+Next = *(ACPI_CAST_INDIRECT_PTR (char,
+(((char *) Curr)[Cache-LinkOffset])));
+if (*(const unsigned char *) Curr != 0xCA) {
+panic(detected use after free %p\n, Curr);
+}
+if (Object == Curr) {
+panic(freeing a free object %p, Object);
+}
+Depth--;
+if (Depth 0) {
+panic(cycle in a cache list);
+}
+}
+
/* Mark the object as cached */
ACPI_MEMSET (Object, 0xCA, Cache-ObjectSize);
@@ -312,6 +334,10 @@
Cache-CurrentDepth--;
+if (*(const unsigned char *) Object != 0xCA) {
+panic(detected use after free %p\n, Object);
+}
+
ACPI_MEM_TRACKING (Cache-Hits++);
ACPI_DEBUG_PRINT ((ACPI_DB_EXEC,
Object %p from %s cache\n, Object, Cache-ListName));
Just in case: this is exactly what I had in mind.
--
Andriy Gapon
___
freebsd-acpi@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
