[PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

Pali Rohár
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 <[hidden email]>
Reviewed-by: Jean Delvare <[hidden email]>
Reported-and-tested-by: Tolga Cakir <[hidden email]>
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: [hidden email] # v4.0+, will need backport
---
 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(&regs) ? : 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;
 
--
1.7.9.5

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

Guenter Roeck-5
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 <[hidden email]>
> Reviewed-by: Jean Delvare <[hidden email]>
> Reported-and-tested-by: Tolga Cakir <[hidden email]>
> 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: [hidden email] # 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(&regs) ? : 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;
>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

Pali Rohár
On Sunday 22 May 2016 02:19:48 Guenter Roeck wrote:

> 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 <[hidden email]>
> > Reviewed-by: Jean Delvare <[hidden email]>
> > Reported-and-tested-by: Tolga Cakir <[hidden email]>
> > 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: [hidden email] # v4.0+, will need backport
>
> Should this patch be applied, or do you wait for more testing ?
I would like to hear some confirmation from people with affected machine.

> 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(&regs) ? : 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;

--
Pali Rohár
[hidden email]

signature.asc (205 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

Thorsten Leemhuis
In reply to this post by Pali Rohár
Lo!

Pali Rohár wrote on 21.05.2016 16:46:
> […] Thorsten […] can you test this patch if it fixes your freeze problem at
> boottime and when using "sensors" program?

FWIW, I never saw either of those problems. I only saw the third issue
that was mentioned: the CPU fan speed is going up and down as described
in https://bugzilla.kernel.org/show_bug.cgi?id=100121

> 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")

Just checked: Sorry, that patch did not fix the CPU fan speed issue on
my XPS 8000, so there is afaics no reason to revert 6220f4ebd7b4 :-/

HTH, CU, knurd

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

Pali Rohár
On Thursday 26 May 2016 17:39:57 Thorsten Leemhuis wrote:
> > 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")
>
> Just checked: Sorry, that patch did not fix the CPU fan speed issue
> on my XPS 8000, so there is afaics no reason to revert 6220f4ebd7b4
> :-/

Please, can you apply patch "dell-smm-hwmon: In debug mode log duration
of SMM calls" and compile dell-smm-hwmon in debug mode to tell us which
SMM call takes too long? This is probably they key for that XPS
problems...

--
Pali Rohár
[hidden email]

signature.asc (205 bytes) Download Attachment
Loading...