[PATCH 7/7] powerpc numa: Consolidate assignment of cpus to nodes
Nathan Lynch
nathanl at austin.ibm.com
Wed Mar 22 06:16:11 EST 2006
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.
More information about the Linuxppc-dev
mailing list