ping ?
Tihs patch is very conservative: it just allow to switch fan OFF if
state is unknown.
Thanks.
--
Sébastien Marie
On Wed, Aug 27, 2014 at 02:51:20PM +0200, Sébastien Marie wrote:
> 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");
> }
> }
