Re: [PATCH 1/8] cpumask: Replace cpumask_of_cpu with cpumask_of_cpu_ptr

[Mike Travis <travis@sgi.com>]

Rusty Russell wrote:
> On Wednesday 16 July 2008 07:14:30 Mike Travis wrote:
>>   * This patch replaces the dangerous lvalue version of cpumask_of_cpu
>>     with new cpumask_of_cpu_ptr macros.  These are patterned after the
>>     node_to_cpumask_ptr macros.
> 
> Hi Mike,
> 
>    Should we just put cpumask_of_cpu_map[] in generic code and then have 
> cpumask_of_cpu() always return a cpumask_t pointer?  These macros which 
> declare things which may be one of two types is a real penalty for code 
> readability.
> 
> Thanks,
> Rusty.

Hi,

I wouldn't mind it at all, and since it's almost always calling a function
that requires a cpumask_t pointer (like the cpu_* ops or set_cpus_allowed_ptr)
then there shouldn't be too many "pointer dereference" penalties.  I'm just
always a bit hesitant to make too many generic changes since I have only x86
and ia64 machines to test with.

But there's a few of these new "fake" pointer macros (well, at least two... ;-),
so we'll either need more of these types of macros, or we have to consider using
pointers for almost all cpumask_t args.  The next jump to 16k cpus will use
2k bytes of stack space for each cpumask_t arg, instead of the current "measly"
512 bytes.

Another thought I had is perhaps cpumask.h should define something that indicates
a "huge NR_CPUS count" that is used globally to trigger things like kmalloc of
cpumask variables, instead of declaring them on the stack...?  Or (as has been
discussed in the past), maybe a new cpumask_t type will be needed?

Thanks,
Mike