Re: [Xen-devel] [PATCH v2] sched: credit2: respect per-vcpu hard affinity

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.03.15 at 04:15, <dario.faggioli@xxxxxxxxxx> wrote:
> On Sun, 2015-02-08 at 17:45 -1000, Justin T. Weaver wrote:
>> --- a/xen/common/sched_credit2.c
>> +++ b/xen/common/sched_credit2.c
>> @@ -194,6 +194,12 @@ int opt_overload_balance_tolerance=-3;
>>  integer_param("credit2_balance_over", opt_overload_balance_tolerance);
>>  
>>  /*
>> + * Use this to avoid having too many cpumask_t structs on the stack
>> + */
>> +static cpumask_t **cpumask = NULL;
>>
> Not just 'cpumask', please... It's too generic a name. Let's pick up
> something that makes it easier to understand what's the purpose of this.
> 
> I'm not really sure right now what something like that could be...
> Credit has balance_mask, but we're not going as far as introducing
> multiple step load balancing here (not with this patch, at least).
> 
> Maybe affinity_cpumask, or hard_affinity_cpumask (and we'll rename to
> something else when introducing soft affinity, if needed).
> 
>> +#define csched2_cpumask cpumask[smp_processor_id()]
>> +
> I like the idea, but put the right side between parentheses.

Parentheses? Why? There's no operator with higher precedence
than postfix ones.

> Of course,
> what just said about the name applies here too. :-)

Right. Suitably done, variable and macro can actually share names.

>> +static int get_safe_pcpu(struct csched2_vcpu *svc)
>> +{
>>
> I also don't like the name... __choose_cpu() maybe ?

Why is everyone liking these double underscore prefixed names so
much? They're in conflict with the library name space and hence
should be avoided. Single underscore prefixed names (and the
underscore not followed by an upper case letter) is what the
standard sets aside for file scope (i.e. static) identifiers.

Jan


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