lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 15 Sep 2012 19:35:18 -0400 (EDT) From: Parag Warudkar <parag.lkml@...il.com> To: Guenter Roeck <linux@...ck-us.net> cc: Parag Warudkar <parag.lkml@...il.com>, lm-sensors@...sensors.org, linux-kernel@...r.kernel.org, rydberg@...omail.se, khali@...ux-fr.org Subject: Re: [PATCH] applesmc: Bump max wait and rearrange udelay On Sat, 15 Sep 2012, Guenter Roeck wrote: > On Sat, Sep 15, 2012 at 06:42:30PM -0400, Parag Warudkar wrote: > > I have been getting a steady stream of wait_read timeouts on my 2010 MBP. > > > > After playing around with various values of APPLESMC_MAX_WAIT a value of > > 0x10000 reduces the wait_read failures to zero under most normal workloads > > - with and without AC power plugged in, at idle and and at make -j4 loads. > > > > While there I noticed we don't really need to udelay before first inb() - > > so I moved it down to after first and subsequent failures. > > > > Been running this for couple days without any issues. > > > > Signed-off-by: Parag Warudkar <parag.lkml@...il.com> > > > > diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c > > index 2827088..46cb458 100644 > > --- a/drivers/hwmon/applesmc.c > > +++ b/drivers/hwmon/applesmc.c > > @@ -56,7 +56,7 @@ > > /* wait up to 32 ms for a status change. */ > > #define APPLESMC_MIN_WAIT 0x0010 > > #define APPLESMC_RETRY_WAIT 0x0100 > > -#define APPLESMC_MAX_WAIT 0x8000 > > +#define APPLESMC_MAX_WAIT 0x10000 > > > > #define APPLESMC_READ_CMD 0x10 > > #define APPLESMC_WRITE_CMD 0x11 > > @@ -170,11 +170,11 @@ static int wait_read(void) > > u8 status; > > int us; > > for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { > > I wonder if it would make sense to keep APPLESMC_MAX_WAIT as it is now > and replace the loop termination conditions with > us <= APPLESMC_MAX_WAIT > > That would accomplish the same, and APPLESMC_MAX_WAIT would reflect > the real maximum wait time. Yeah, that would fix the code to match what it says and eliminate most failures. But in my testing sometimes it needed a max wait of 57344 us - I wrote a separate patch to instrument it by bumping up max wait in increments of 0x2000 until it no longer failed under my normal workloads. Under normal use 32us seemed to work but rarely when machine was slow - throttled, on battery, on kernel build load etc.- the additional wait helped. So I think it makes sense to fix the loop termination condition and also bump to max wait to 0x10000. > > Also, since the delay time can get quite large, would it make sense to replace > udelay with usleep_range() ? > Yes, I think that would be a good thing to do. We could sleep in range of us<<=1 and us<<1 and if usleep_range() returns actual sleep time we can factor that in for next loop iteration if necessary. Gotta think a bit on that one. I will rework the patch to fix the loop termination and keep the bump to 0x10000 in place and possibly also experiment with usleep_range(). Thanks, Parag -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists