From owner-freebsd-acpi@FreeBSD.ORG Wed Apr 15 17:29:45 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 C31ED1065761; Wed, 15 Apr 2009 17:29:45 +0000 (UTC) (envelope-from nate@root.org) Received: from nlpi053.prodigy.net (nlpi053.sbcis.sbc.com [207.115.36.82]) by mx1.freebsd.org (Postfix) with ESMTP id C04238FC25; Wed, 15 Apr 2009 17:29:44 +0000 (UTC) (envelope-from nate@root.org) Received: from [10.0.5.18] (ppp-71-139-9-173.dsl.snfc21.pacbell.net [71.139.9.173]) (authenticated bits=0) by nlpi053.prodigy.net (8.13.8 smtpauth/dk/map_regex/8.13.8) with ESMTP id n3FHTghv025874; Wed, 15 Apr 2009 12:29:43 -0500 Message-ID: <49E61986.7040709@root.org> Date: Wed, 15 Apr 2009 10:29:42 -0700 From: Nate Lawson User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) MIME-Version: 1.0 To: Andriy Gapon References: <49DB639A.4090504@icyb.net.ua> <49DCF5C2.60805@root.org> <49DDF906.8090400@icyb.net.ua> <49DF3CA4.1090309@freebsd.org> <49E4B2A7.3020302@freebsd.org> In-Reply-To: <49E4B2A7.3020302@freebsd.org> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=UTF-8 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 17:29:46 -0000 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. > > 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. Overall, still looks good. I like the idea of gotos instead of the do/while(0). It's ok if the indent changes. > 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 trust jkim's opinion also. So it is probably ok to leave this way. Make sure someone with SMP tests that their system still powers off ok with this patch. Thanks, -- Nate