Re: [PATCH v5 2/8] xenalyze: increase NR_CPUS to 256

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/06/15 17:14, Olaf Hering wrote:
> On Thu, Jun 11, George Dunlap wrote:
>
>> On 06/11/2015 12:03 PM, Julien Grall wrote:
>>> I would suggest some refactoring to remove NR_CPUS and associated code
>>> in order to avoid mis-usage later.
>>>
>>> Also, cpu_mask_t is a uint32_t, is it intentional?
>> When xenalyze was originally written back in 2006, I think that's
>> probably how it was defined in Xen.  I just haven't used it on a system
>> with more than 32 cpus. :-)
>>
>> That certainly should be addressed at some point.  I'd be in favor of
>> checking this in and fixing it up later; or, for the time being, just
>> disabling the skew detection functionality (which is the only place that
>> uses it, AFAICT).
> Today I looked at this part:
>
> In v6 I have added a patch to remove cpumask_t. Now I see xenalyze.c has
> a cpu_mask_t with just 32 bits. Easy change would be to change that to
> uin64_t.
>
> To really support MAX_CPUS the code can either include
> xen/include/xen/cpumask.h, or just the few required functions can be
> pasted into xenalyze.c.
>
> What do you prefer?

In an ideal world, userspace tools like this should not really be tied
to NR_CPUS or MAX_CPUS.  They should get max_cpu_id from Xen and
dynamically allocate a bitmap of sufficient size.

Whether this is a yak worth shaving now is a different matter.

~Andrew