From owner-freebsd-acpi@FreeBSD.ORG Wed Apr 15 08:59:49 2009 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BDF94106566B for ; Wed, 15 Apr 2009 08:59:49 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id C8B1E8FC22 for ; Wed, 15 Apr 2009 08:59:48 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from porto.topspin.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id LAA09108; Wed, 15 Apr 2009 11:59:47 +0300 (EEST) (envelope-from avg@freebsd.org) Received: from localhost.topspin.kiev.ua ([127.0.0.1] helo=edge.pp.kiev.ua) by porto.topspin.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1Lu0yB-000AJE-0P; Wed, 15 Apr 2009 11:59:47 +0300 Message-ID: <49E5A200.6010306@freebsd.org> Date: Wed, 15 Apr 2009 11:59:44 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.19 (X11/20090114) MIME-Version: 1.0 To: Jung-uk Kim References: <49DB639A.4090504@icyb.net.ua> <49DF3CA4.1090309@freebsd.org> <49E4B2A7.3020302@freebsd.org> <200904141424.00943.jkim@FreeBSD.org> In-Reply-To: <200904141424.00943.jkim@FreeBSD.org> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@freebsd.org Subject: Re: run resume code only for S1-S4 states X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Apr 2009 08:59:50 -0000 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