kern/108581: [sysctl] sysctl: hw.acpi.cpu.cx_lowest: Invalid argument

[Stephane E. Potvin]

Nate Lawson wrote:
> Stephane E. Potvin wrote:
>> Andriy Gapon wrote:
>>> on 26/03/2009 17:10 Bruce Cran said the following:
>>>> On Thu, 26 Mar 2009 17:04:19 +0200
>>>> Andriy Gapon <avg at icyb.net.ua> wrote:
>>>>
>>>>> on 26/03/2009 16:41 Bruce Cran said the following:
>>>>>> I added lots of printfs to acpi_cpu.c and found that it's occuring
>>>>>> in acpi_cpu_startup; initializing it to 3 in that function (which I
>>>>>> wrongly assumed was the lowest Cx state supported in ACPI) fixed
>>>>>> the problem on my Athlon XP PC because the generic cx handling code
>>>>>> then lowered cpu_cx_count to 1 based on the fact that
>>>>>> sc->cpu_cx_count was also 1.
>>>>>>
>>>>> Ok, yes, the real issue is in acpi_cpu_generic_cx_probe, namely in
>>>>> early exits from it. So,  sc->cpu_cx_count is always set to at least
>>>>> 1, but if we exit via one of the returns before the end of function,
>>>>> then global cpu_cx_count is never updated.
>>>>>
>>>> Exactly:
>>>>
>>>> acpi: acpi_cpu_startup: initializing cpu_cx_count to 0
>>>> acpi_cpu_generic_cx_probe
>>>> if sc->cpu_p_blk_len < 5  [sc->cpu_p_blk_len = 0]
>>>> acpi: acpi_cpu_startup: cpu 0,cpu_cx_count = 0,sc->cpu_cx_count = 1
>>>>
>>>> So we're hitting an early exit in acpi_cpu_generic_cx_probe.
>>>>
>>> John, what would be a better fix - initialize the global variable to 1 or use goto
>>> in acpi_cpu_generic_cx_probe?
>>> I think the latter is more consistent and obvious, the former is simpler and
>>> safer, though.
>>>
>> Your right, it seems that I need to order some more pointy hats. There
>> should have been a goto there to jump at the end in order to initialize
>> the global cpu_cx_count. The following patch should fix your issue.
>> John, is this ok with you?
> 
> John's patch does the same thing without a goto (see message
> <200903261129.50419.jhb at freebsd.org>)
> 
I saw it after sending mine, John's patch is indeed better.

Steph