Re: [RFC PATCH 1/1] fair.c: Add/Export find_idlest_perfer_cpu API

[Peter Zijlstra <peterz@infradead.org>]

On Mon, 2012-08-20 at 15:17 -0700, Shirley Ma wrote:
> On Mon, 2012-08-20 at 14:00 +0200, Peter Zijlstra wrote:
> > On Fri, 2012-08-17 at 12:46 -0700, Shirley Ma wrote:
> > > Add/Export a new API for per-cpu thread model networking device
> > driver
> > > to choose a preferred idlest cpu within allowed cpumask.
> > > 
> > > The receiving CPUs of a networking device are not under cgroup
> > controls.
> > > Normally the receiving work will be scheduled on the cpu on which
> > the
> > > interrupts are received. When such a networking device uses per-cpu
> > > thread model, the cpu which is chose to process the packets might
> > not be
> > > part of cgroup cpusets without using such an API here. 
> > > 
> > > On NUMA system, by using the preferred cpumask from the same NUMA
> > node
> > > would help to reduce expensive cross memory access to/from the other
> > > NUMA node.
> > > 
> > > KVM per-cpu vhost will be the first one to use this API. Any other
> > > device driver which uses per-cpu thread model and has cgroup cpuset
> > > control will use this API later.
> > 
> > How often will this be called and how do you obtain the cpumasks
> > provided to the function? 
> 
> It depends. It might be called pretty often if the user keeps changing
> cgroups control cpuset. It might be less called if the cgroups control
> cpuset is stable, and the host scheduler always schedules the work on
> the same NUMA node.

This just doesn't make any sense, you're scanning for the least loaded
cpu, this is unrelated to a change in cpuset. So tying the scan
frequency to changes in configuration is just broken.

> The preferred cpumasks are obtained from local numa node.

So why pass it as argument at all? Also, who says the current node is
the right one? It might just be running there temporarily.

>  The allowed
> cpumasks are obtained from caller's task allowed cpumasks (cgroups
> control cpuset).

task->cpus_allowed != cpusets.. Also, since you're using
task->cpus_allowed, pass a task_struct *, not a cpumask.