run resume code only for S1-S4 states

Andriy Gapon avg at freebsd.org
Wed Apr 15 01:59:50 PDT 2009


on 14/04/2009 21:23 Jung-uk Kim said the following:
> On Tuesday 14 April 2009 11:58 am, Andriy Gapon wrote:
>> Guys,
>> could you please review the attached patch?
>>
>> Its main idea is to make control flow of acpi_EnterSleepState
>> similar to that of acpi_ReqSleepState: reject invalid state
>> parameter immediately and handle special S5 as early as possible.
>> Primary purpose is to avoid running resume code when it is not
>> necessary - e.g. shutdown_nice() typically returns immediately
>> after initiating a graceful shutdown by sending a signal to init.
> 
> I tried to solve this problem once.  To preserve the current 
> behaviour, you have to clean up sc->acpi_next_sstate and set 
> sc->acpi_sstate to S5 as well if my memory serves.

I am not sure if I understand why/where this could be useful.
S5 is a "terminal" state, so unless shutdown fails for some reason (can
there be any?) this shouldn't matter.

>> As such, S5 is handled right after checking/disabling re-entry.
>> switch becomes unneeded, because all remaining possibilities are
>> grouped into a single case. I decided to use do-while(0) statement
>> in the place of the switch for the following reasons:
>> 1. minimize diff by preserving indentation
>> 2. minimize diff by preserving control flow that depends on break
>> statement But I am not sure how this while(0) corresponds with
>> style(9), I couldn't find any reference in the manual page.
> 
> I think goto is more cleaner and easy to read in this case.

Ok, I'll switch to that.

>> There is also a concern about calling shutdown_nice() outside of
>> the Giant lock and binding to CPU 0. I am not sure about the
>> pre-requisites for this function. John, maybe you could help me
>> here?
> 
> I think you don't need giant here and CPU binding is done from boot().

Thank you for the review!

-- 
Andriy Gapon


More information about the freebsd-acpi mailing list