Date: Wed, 15 Apr 2009 11:59:44 +0300 From: Andriy Gapon <avg@freebsd.org> To: Jung-uk Kim <jkim@freebsd.org> Cc: freebsd-acpi@freebsd.org Subject: Re: run resume code only for S1-S4 states Message-ID: <49E5A200.6010306@freebsd.org> In-Reply-To: <200904141424.00943.jkim@FreeBSD.org> References: <49DB639A.4090504@icyb.net.ua> <49DF3CA4.1090309@freebsd.org> <49E4B2A7.3020302@freebsd.org> <200904141424.00943.jkim@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49E5A200.6010306>