Hi Jonathan,
First, thanks for your feedback and for your patch.
On Wed, Aug 27, 2014 at 02:42:43AM +1000, Jonathan Gray wrote:
> Rather than calling acpi_setfan() again would it make
> sense to remove the cached state value and move
> the state reading to just before the method is called?
>
> That said I'm not familiar with the acpitz code and
> haven't gone off to look at the spec.
At first reading I am quite disappointed to remove caching.
So I have:
- checked in others acpi termal-zone implementation to see if caching
is using or not for active cooling.
- try to measure some values on my host.
Others implementations checked:
- netbsd : caching used (sys/dev/acpi/acpi_tz.c, l.368)
- freebsd : caching used (head/sys/dev/acpica/acpi_thermal.c, l.564)
- linux : caching seems not be used (but not 100% sure)
(please note I am not expert in theses kernels implementations, so I
could be wrong).
Some measures:
I first try to measure the cost of acpi_setfan in term of time:
something between 9998432ns et 14574920ns (0.0099 and 0.015 s). It
seems not being a heavy operation (too my eyes).
Secondly, the number of call of acpi_setfan, with and without caching.
The test kernel is build with caching enable. The without-caching
counter is incremented every time acpi_setfan would be call without
caching, and with-caching counter incremented only when acpi_setfan is
called (the patch below was applied: an unknown state would result
acpi_setfan calling).
During 1h09 :
- without caching: 3388 calls
- with caching: 742 calls
The cache is used at 78% of time.
So, even if your patch resolve the initial problem (the fan don't keep
running whereas state is unknown), I would prefer to keep the cache in
place. But if you think the impact of not use this cache is negligible,
it is ok for me.
I join a very conservative patch which just allow calling
acpi_setfan(sc,i,"_OFF") if cache is unknown.
Thanks.
--
Sébastien Marie
Index: dev/acpi/acpitz.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpitz.c,v
retrieving revision 1.47
diff -u -p -r1.47 acpitz.c
--- dev/acpi/acpitz.c 12 Jul 2014 02:44:49 -0000 1.47
+++ dev/acpi/acpitz.c 27 Aug 2014 12:38:36 -0000
@@ -424,7 +424,7 @@ acpitz_refresh(void *arg)
acpitz_setfan(sc, i, "_ON_");
} else if (sc->sc_ac[i] != -1) {
/* turn off fan i */
- if (sc->sc_ac_stat[i] > 0)
+ if (sc->sc_ac_stat[i] == -1 || sc->sc_ac_stat[i] > 0)
acpitz_setfan(sc, i, "_OFF");
}
}
