From owner-freebsd-acpi@FreeBSD.ORG Thu Mar 26 15:33:58 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 EA260106564A for ; Thu, 26 Mar 2009 15:33:58 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 7FDCF8FC0A for ; Thu, 26 Mar 2009 15:33:58 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (pool-98-109-39-197.nwrknj.fios.verizon.net [98.109.39.197]) by cyrus.watson.org (Postfix) with ESMTPSA id EA86A46B58; Thu, 26 Mar 2009 11:33:57 -0400 (EDT) Received: from localhost (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.14.3/8.14.3) with ESMTP id n2QFXpS0083482; Thu, 26 Mar 2009 11:33:52 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Andriy Gapon Date: Thu, 26 Mar 2009 11:29:50 -0400 User-Agent: KMail/1.9.7 References: <200903200030.n2K0U3iG011009@freefall.freebsd.org> <20090326151035.51e4196e@gluon.draftnet> <49CB9C1B.4070308@icyb.net.ua> In-Reply-To: <49CB9C1B.4070308@icyb.net.ua> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200903261129.50419.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Thu, 26 Mar 2009 11:33:52 -0400 (EDT) X-Virus-Scanned: ClamAV 0.94.2/9169/Thu Mar 26 00:13:48 2009 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: freebsd-acpi@freebsd.org Subject: Re: kern/108581: [sysctl] sysctl: hw.acpi.cpu.cx_lowest: Invalid argument 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: Thu, 26 Mar 2009 15:33:59 -0000 On Thursday 26 March 2009 11:15:39 am Andriy Gapon wrote: > on 26/03/2009 17:10 Bruce Cran said the following: > > On Thu, 26 Mar 2009 17:04:19 +0200 > > Andriy Gapon wrote: > > > >> on 26/03/2009 16:41 Bruce Cran said the following: > > > >>> I added lots of printfs to acpi_cpu.c and found that it's occuring > >>> in acpi_cpu_startup; initializing it to 3 in that function (which I > >>> wrongly assumed was the lowest Cx state supported in ACPI) fixed > >>> the problem on my Athlon XP PC because the generic cx handling code > >>> then lowered cpu_cx_count to 1 based on the fact that > >>> sc->cpu_cx_count was also 1. > >>> > >> Ok, yes, the real issue is in acpi_cpu_generic_cx_probe, namely in > >> early exits from it. So, sc->cpu_cx_count is always set to at least > >> 1, but if we exit via one of the returns before the end of function, > >> then global cpu_cx_count is never updated. > >> > > > > Exactly: > > > > acpi: acpi_cpu_startup: initializing cpu_cx_count to 0 > > acpi_cpu_generic_cx_probe > > if sc->cpu_p_blk_len < 5 [sc->cpu_p_blk_len = 0] > > acpi: acpi_cpu_startup: cpu 0,cpu_cx_count = 0,sc->cpu_cx_count = 1 > > > > So we're hitting an early exit in acpi_cpu_generic_cx_probe. > > > > John, what would be a better fix - initialize the global variable to 1 or use goto > in acpi_cpu_generic_cx_probe? > I think the latter is more consistent and obvious, the former is simpler and > safer, though. I would rather move the cpu_cx_count code out into acpi_cpu_startup() completely. It would more closely match the _CST code in that case. It is also easier to follow the logic this way as well as it is only modified in one place and not via a secret side-effect. --- //depot/vendor/freebsd/src/sys/dev/acpica/acpi_cpu.c 2009/02/19 14:40:18 +++ //depot/user/jhb/acpipci/dev/acpica/acpi_cpu.c 2009/03/26 15:28:32 @@ -609,10 +609,6 @@ sc->cpu_cx_count++; } } - - /* Update the largest cx_count seen so far */ - if (sc->cpu_cx_count > cpu_cx_count) - cpu_cx_count = sc->cpu_cx_count; } /* @@ -752,6 +748,8 @@ for (i = 0; i < cpu_ndevices; i++) { sc = device_get_softc(cpu_devices[i]); acpi_cpu_generic_cx_probe(sc); + if (sc->cpu_cx_count > cpu_cx_count) + cpu_cx_count = sc->cpu_cx_count; } /* -- John Baldwin