On Mon, May 07, 2018 at 05:01:34PM -0400, Johannes Weiner wrote:
> +static void psi_clock(struct work_struct *work)
> +{
> +     u64 some[NR_PSI_RESOURCES] = { 0, };
> +     u64 full[NR_PSI_RESOURCES] = { 0, };
> +     unsigned long nonidle_total = 0;
> +     unsigned long missed_periods;
> +     struct delayed_work *dwork;
> +     struct psi_group *group;
> +     unsigned long expires;
> +     int cpu;
> +     int r;
> +
> +     dwork = to_delayed_work(work);
> +     group = container_of(dwork, struct psi_group, clock_work);
> +
> +     /*
> +      * Calculate the sampling period. The clock might have been
> +      * stopped for a while.
> +      */
> +     expires = group->period_expires;
> +     missed_periods = (jiffies - expires) / MY_LOAD_FREQ;
> +     group->period_expires = expires + ((1 + missed_periods) * MY_LOAD_FREQ);
> +
> +     /*
> +      * Aggregate the per-cpu state into a global state. Each CPU
> +      * is weighted by its non-idle time in the sampling period.
> +      */
> +     for_each_online_cpu(cpu) {

Typically when using online CPU state, you also need hotplug notifiers
to deal with changes in the online set.

You also typically need something like cpus_read_lock() around an
iteration of online CPUs, to avoid the set changing while you're poking
at them.

The lack for neither is evident or explained.

> +             struct psi_group_cpu *groupc = per_cpu_ptr(group->cpus, cpu);
> +             unsigned long nonidle;
> +
> +             nonidle = nsecs_to_jiffies(groupc->nonidle_time);
> +             groupc->nonidle_time = 0;
> +             nonidle_total += nonidle;
> +
> +             for (r = 0; r < NR_PSI_RESOURCES; r++) {
> +                     struct psi_resource *res = &groupc->res[r];
> +
> +                     some[r] += (res->times[0] + res->times[1]) * nonidle;
> +                     full[r] += res->times[1] * nonidle;
> +
> +                     /* It's racy, but we can tolerate some error */
> +                     res->times[0] = 0;
> +                     res->times[1] = 0;
> +             }
> +     }
> +
> +     for (r = 0; r < NR_PSI_RESOURCES; r++) {
> +             /* Finish the weighted aggregation */
> +             some[r] /= max(nonidle_total, 1UL);
> +             full[r] /= max(nonidle_total, 1UL);
> +
> +             /* Accumulate stall time */
> +             group->some[r] += some[r];
> +             group->full[r] += full[r];
> +
> +             /* Calculate recent pressure averages */
> +             calc_avgs(group->avg_some[r], some[r], missed_periods);
> +             calc_avgs(group->avg_full[r], full[r], missed_periods);
> +     }
> +
> +     /* Keep the clock ticking only when there is action */
> +     if (nonidle_total)
> +             schedule_delayed_work(dwork, MY_LOAD_FREQ);
> +}

Reply via email to