Re: [PATCH 7/7] powerpc numa: Consolidate assignment of cpus to nodes

[Nathan Lynch <nathanl@austin.ibm.com>]

On Tue, 2006-03-21 at 10:38 -0800, Dave Hansen wrote:
> On Mon, 2006-03-20 at 18:37 -0600, Nathan Lynch wrote:
> > +       cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
> > +                         (void *)(unsigned long)boot_cpuid); 
> 
> That double-cast really caught my eye.  cpu_numa_callback() looks a
> little bit confused about what type cpuids should be.
> 
> Its lcpu is an "unsigned long", but it has integers passed into it
> (boot_cpuid), and calls map_cpu_to_node(lcpu, 0), where the first
> argument is an integer, but an "unsigned long" is passed in.  This may
> be harmless, but I still have to think about it, which is bad.
> 
> Seems like just making cpu_numa_callback()'s lcpu an int would get rid
> of at least one net cast.  Why not just pass &boot_cpuid in there, and
> do this:
> 
> 	int lcpu = *(int *)hcpu;

That's not the convention for cpu hotplug notifiers.  The id of the cpu
subject to online/offline is passed in the void * argument.  I'd have to
change the cpu hotplug core and every notifier in the kernel to
implement your suggestion.

> 
> That makes it _really_ obvious what is going on.  While it isn't
> horribly uncommon to pass integers around inside of void*s, it can be a
> bit confusing.  You also get readability issues with long<->int
> conversions as you saw.
> 
> By the way, what do the "l" and "h" in front of "cpu" mean anyway?

"logical" and "hot"?  I dunno, just seemed to be the convention in other
cpu notifiers at the time the code was written.