[rfc] bind curthread to target cpu for _CST change notification

[Andriy Gapon]

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