Re: [PATCH v4 7/7] cpufreq: schedutil: New governor based on scheduler utilization data

From: Peter Zijlstra
Date: Wed Mar 16 2016 - 13:52:18 EST


On Wed, Mar 16, 2016 at 03:59:18PM +0100, Rafael J. Wysocki wrote:
> +static void sugov_work(struct work_struct *work)
> +{
> + struct sugov_policy *sg_policy = container_of(work, struct sugov_policy, work);
> +
> + mutex_lock(&sg_policy->work_lock);
> + __cpufreq_driver_target(sg_policy->policy, sg_policy->next_freq,
> + CPUFREQ_RELATION_L);
> + mutex_unlock(&sg_policy->work_lock);
> +

Be aware that the below store can creep up and become visible before the
unlock. AFAICT that doesn't really matter, but still.

> + sg_policy->work_in_progress = false;
> +}
> +
> +static void sugov_irq_work(struct irq_work *irq_work)
> +{
> + struct sugov_policy *sg_policy;
> +
> + sg_policy = container_of(irq_work, struct sugov_policy, irq_work);
> + schedule_work(&sg_policy->work);
> +}

If you care what cpu the work runs on, you should schedule_work_on(),
regular schedule_work() can end up on any random cpu (although typically
it does not).

In particular schedule_work() -> queue_work() -> queue_work_on(.cpu =
WORK_CPU_UNBOUND) -> __queue_work() if (req_cpu == UNBOUND) cpu =
wq_select_unbound_cpu(), which has a Round-Robin 'feature' to detect
just such dependencies.