Re: Alternate cpu policy on battery

[Stefan Sperling]

On Sat, Sep 25, 2021 at 08:26:06PM +0200, Solene Rapenne wrote:
> +void
> +setperf_powersaving(void *v)
> +{
> +     static uint64_t *idleticks, *totalticks;

[...]

> +
> +     if (!idleticks)
> +             if (!(idleticks = mallocarray(ncpusfound, sizeof(*idleticks),
> +                 M_DEVBUF, M_NOWAIT | M_ZERO)))
> +                     return;
> +     if (!totalticks)
> +             if (!(totalticks = mallocarray(ncpusfound, sizeof(*totalticks),
> +                 M_DEVBUF, M_NOWAIT | M_ZERO))) {
> +                     free(idleticks, M_DEVBUF,
> +                         sizeof(*idleticks) * ncpusfound);
> +                     return;
> +             }

This is unrelated to your patch.
I noticed something that looks wrong in code your patch was based on.

Should setperf_auto() not reset idleticks to NULL after freeing idleticks
in the error path of the totalticks allocation?

Otherwise, if we come back into this function a second time, the static(!)
variable idleticks will still point at freed memory and won't be reallocated
since !idleticks is now false.

A very unlikely edge case. Still worth fixing?

diff ba3fb53d3e94158190d84a1771da21f620e46e26 /usr/src
blob - 956ea24ea07fa9a7301f2e7156698e4e090e9cfb
file + sys/kern/sched_bsd.c
--- sys/kern/sched_bsd.c
+++ sys/kern/sched_bsd.c
@@ -565,10 +565,11 @@ setperf_auto(void *v)
        if (!totalticks)
                if (!(totalticks = mallocarray(ncpusfound, sizeof(*totalticks),
                    M_DEVBUF, M_NOWAIT | M_ZERO))) {
                        free(idleticks, M_DEVBUF,
                            sizeof(*idleticks) * ncpusfound);
+                       idleticks = NULL;
                        return;
                }
 
        alltotal = allidle = 0;
        j = 0;