From owner-freebsd-acpi@FreeBSD.ORG Thu Mar 26 15:40:24 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 BE70F1065670 for ; Thu, 26 Mar 2009 15:40:24 +0000 (UTC) (envelope-from sepotvin@FreeBSD.org) Received: from zerofail.com (gatekeeper1.zerofail.com [208.71.11.38]) by mx1.freebsd.org (Postfix) with ESMTP id 768FA8FC20 for ; Thu, 26 Mar 2009 15:40:24 +0000 (UTC) (envelope-from sepotvin@FreeBSD.org) Received: from telcobridges.com by freebsd.org (zerofail.com) (SecurityGateway 1.1.4) with SMTP id SG002297095.MSG for ; Thu, 26 Mar 2009 11:40:22 -0400 Received: from leia.telcobridges.lan ([208.94.105.59]) by telcobridges.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 26 Mar 2009 11:40:21 -0400 Message-ID: <49CBA1E5.2090902@FreeBSD.org> Date: Thu, 26 Mar 2009 11:40:21 -0400 From: "Stephane E. Potvin" Organization: FreeBSD Project User-Agent: Thunderbird 2.0.0.21 (X11/20090323) MIME-Version: 1.0 To: Andriy Gapon References: <200903200030.n2K0U3iG011009@freefall.freebsd.org> <20090325223914.4387eeae@gluon.draftnet> <49CB8C86.4020800@icyb.net.ua> <20090326142832.0dba187a@gluon.draftnet> <49CB9224.6010509@icyb.net.ua> <20090326144140.2203c0d8@gluon.draftnet> <49CB9973.3010306@icyb.net.ua> <20090326151035.51e4196e@gluon.draftnet> <49CB9C1B.4070308@icyb.net.ua> In-Reply-To: <49CB9C1B.4070308@icyb.net.ua> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 26 Mar 2009 15:40:21.0344 (UTC) FILETIME=[2C181E00:01C9AE29] X-SGHeloLookup-Result: hardfail smtp.helo=telcobridges.com (does not match 208.71.8.41) X-SGOP-RefID: str=0001.0A090205.49CBA1E5.01E8,ss=1,fgs=0 (_st=1 _vt=0 _iwf=0) 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:40:25 -0000 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. > Your right, it seems that I need to order some more pointy hats. There should have been a goto there to jump at the end in order to initialize the global cpu_cx_count. The following patch should fix your issue. John, is this ok with you? Index: acpi_cpu.c =================================================================== --- acpi_cpu.c (revision 190318) +++ acpi_cpu.c (working copy) @@ -576,7 +576,7 @@ * "only" C1-C3 is not a hardship. */ if (sc->cpu_p_blk_len < 5) - return; + goto done; /* Validate and allocate resources for C2 (P_LVL2). */ gas.SpaceId = ACPI_ADR_SPACE_SYSTEM_IO; @@ -594,7 +594,7 @@ } } if (sc->cpu_p_blk_len < 6) - return; + goto done; /* Validate and allocate resources for C3 (P_LVL3). */ if (AcpiGbl_FADT.C3Latency <= 1000 && !(cpu_quirks & CPU_QUIRK_NO_C3)) { @@ -610,6 +610,7 @@ } } +done: /* Update the largest cx_count seen so far */ if (sc->cpu_cx_count > cpu_cx_count) cpu_cx_count = sc->cpu_cx_count;