From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752042AbcEVAUS (ORCPT ); Sat, 21 May 2016 20:20:18 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:38479 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751924AbcEVAUM (ORCPT ); Sat, 21 May 2016 20:20:12 -0400 Subject: Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection To: =?UTF-8?Q?Pali_Roh=c3=a1r?= , Jan C Peters , Thorsten Leemhuis , =?UTF-8?Q?David_Santamar=c3=ada_Rogado?= , Peter Saunderson , Jean Delvare , Tolga Cakir References: <1463842001-17785-1-git-send-email-pali.rohar@gmail.com> Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org From: Guenter Roeck Message-ID: <5740FB24.80709@roeck-us.net> Date: Sat, 21 May 2016 17:19:48 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1463842001-17785-1-git-send-email-pali.rohar@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2016 07:46 AM, Pali Rohár wrote: > On more Dell machines (e.g. Dell Precision M3800) I8K_SMM_GET_FAN_TYPE call > is too expensive (CPU is too long in SMM mode) and cause kernel to hang. > This patch cache type for each fan (as it should not change) and change the > way how fan presense is detected. It revert and use function fan_status() > as was before commit f989e55452c7 ("i8k: Add support for fan labels"). > > Moreover, kernel hangs for 2 - 3 seconds only sometimes and only on some > Dell machines. When kernel hangs fan speed is at max. So it was hard to > debug and bisect where is root of this problem. It looks like this is bug > in Dell BIOS which implement fan type SMM code... and there is no way how > to fix it in kernel. > > Signed-off-by: Pali Rohár > Reviewed-by: Jean Delvare > Reported-and-tested-by: Tolga Cakir > Fixes: f989e55452c7 ("i8k: Add support for fan labels") > Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121 > Cc: stable@vger.kernel.org # v4.0+, will need backport Should this patch be applied, or do you wait for more testing ? Guenter > --- > drivers/hwmon/dell-smm-hwmon.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > --- > > Hi! > > Jan C Peters, Thorsten Leemhuis, David Santamaría Rogado, Peter Saunderson > and others, can you test this patch if it fixes your freeze problem at > boottime and when using "sensors" program? > > I need to know if this patch fixes problem on Dell Studio XPS 8000 and > Dell Studio XPS 8100 machines, so we can revert git commits: > > 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000") > a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100") > > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c > index c43318d..577445f 100644 > --- a/drivers/hwmon/dell-smm-hwmon.c > +++ b/drivers/hwmon/dell-smm-hwmon.c > @@ -235,7 +235,7 @@ static int i8k_get_fan_speed(int fan) > /* > * Read the fan type. > */ > -static int i8k_get_fan_type(int fan) > +static int _i8k_get_fan_type(int fan) > { > struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, }; > > @@ -243,6 +243,17 @@ static int i8k_get_fan_type(int fan) > return i8k_smm(®s) ? : regs.eax & 0xff; > } > > +static int i8k_get_fan_type(int fan) > +{ > + /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */ > + static int types[2] = { INT_MIN, INT_MIN }; > + > + if (types[fan] == INT_MIN) > + types[fan] = _i8k_get_fan_type(fan); > + > + return types[fan]; > +} > + > /* > * Read the fan nominal rpm for specific fan speed. > */ > @@ -767,13 +778,13 @@ static int __init i8k_init_hwmon(void) > if (err >= 0) > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4; > > - /* First fan attributes, if fan type is OK */ > - err = i8k_get_fan_type(0); > + /* First fan attributes, if fan status is OK */ > + err = i8k_get_fan_status(0); > if (err >= 0) > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1; > > - /* Second fan attributes, if fan type is OK */ > - err = i8k_get_fan_type(1); > + /* Second fan attributes, if fan status is OK */ > + err = i8k_get_fan_status(1); > if (err >= 0) > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; > >