Messages in this thread | | | Date | Fri, 18 Jul 2008 06:43:07 -0700 | From | Mike Travis <> | Subject | Re: [PATCH 1/8] cpumask: Replace cpumask_of_cpu with cpumask_of_cpu_ptr |
| |
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
| |