Re: [Xen-devel] [PATCH 03 of 11 v4] xen: sched_credit: when picking, make sure we get an idle one, if any

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.13 at 11:37, Dario Faggioli <dario.faggioli@xxxxxxxxxx> wrote:
> On ven, 2013-03-15 at 08:14 +0000, Jan Beulich wrote:
> Perhaps I can turn the condition into something like this:
> 
>     if ( !cpumask_test_cpu(cpu, &cpus) )
>         cpu = cpumask_empty(&cpus) ? cpu : cpumask_cycle(cpu, &cpus);
> 
> So that we pay the price less frequently?

Given cpu < nr_cpu_ids before this, yes, that sounds right. Or
you could simply switch the operands of the && in your original
if(). Yet less expensive would be if you stored the result of
cpumask_cycle() in another variable and copied it into "cpu"
only if less than nr_cpu_ids. That would eliminate the need for
cpumask_empty() altogether.

>> The ASSERT() a few lines earlier
>> could be simplified in similar ways, btw.
>> 
> You mean this, right?
> 
>     online = cpupool_scheduler_cpumask(vc->domain->cpupool);
>     cpumask_and(&cpus, online, vc->cpu_affinity);
>     cpu = cpumask_test_cpu(vc->processor, &cpus)
>             ? vc->processor
>             : cpumask_cycle(vc->processor, &cpus);
>     ASSERT( !cpumask_empty(&cpus) && cpumask_test_cpu(cpu, &cpus) );
> 
> Not sure. AFAIU the code, the ASSERT() is indeed willing to make sure
> that cpus did not ended up being empty as a consequence of the
> cpumask_and(), and that is done together with the cpumask_test_cpu()
> just to have only one ASSERT() instead of two, but again, I might well
> be wrong.

If I'm not mistaken,

    ASSERT(cpu < nr_cpu_ids && cpumask_test_cpu(cpu, &cpus));

would have the exact same effect.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel