Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Apr 2009 10:29:42 -0700
From:      Nate Lawson <nate@root.org>
To:        Andriy Gapon <avg@freebsd.org>
Cc:        freebsd-acpi@freebsd.org
Subject:   Re: run resume code only for S1-S4 states
Message-ID:  <49E61986.7040709@root.org>
In-Reply-To: <49E4B2A7.3020302@freebsd.org>
References:  <49DB639A.4090504@icyb.net.ua> <49DCF5C2.60805@root.org>	<49DDF906.8090400@icyb.net.ua> <49DF3CA4.1090309@freebsd.org> <49E4B2A7.3020302@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49E61986.7040709>