Re: [PATCH] x86-64, NUMA: fix fakenuma boot failure

[Tejun Heo <tj@kernel.org>]

Hello,

On Wed, Apr 13, 2011 at 04:02:43PM +0900, KOSAKI Motohiro wrote:
> Your patch have two mistake.
> 
>  1) link_thread_siblings() is for HT
>     set_cpu_sibling_map() has another sibling calculations.
>  2) numa_set_node() is not enough. scheduler is using node_to_cpumask_map[] too.

Thanks for seeing this through but your patch is badly whitespace
broken.  Can you please check your mail setup and repost?  Also, some
comments below.

> btw, Please see cpu_coregroup_mask(). its return value depend on 
> sched_mc_power_savings and sched_smt_power_savings. then, we need to care
> both cpu_core_mask and cpu_llc_shared_mask. I think.

Hmmmm....

> +static void __cpuinit node_cpumap_same_phys(int cpu1, int cpu2)

What does the "phys" mean?  Maybe something like
check_cpu_siblings_on_same_node() is a better name?

> +	/*
> +	 * Our CPU scheduler assume all cpus in the same physical cpu package
> +	 * are assigned the same node. But, Buggy ACPI table or NUMA emulation
> +	 * might assigne them to different node. Fix it.
		typo

> +	*/
> +	if (node1 != node2) {
> +		pr_warning("CPU %d in node %d and CPU %d in node %d are in the same physical CPU. forcing same node %d\n",
> +			   cpu1, node1, cpu2, node2, node2);
> +
> +		numa_set_node(cpu1, node2);
> +		cpumask_set_cpu(cpu1, node_to_cpumask_map[node2]);
> +		cpumask_clear_cpu(cpu1, node_to_cpumask_map[node1]);

Maybe what you want is the following?

	numa_remove_cpu(cpu1);
	numa_set_node(cpu1, node2)
	numa_add_cpu(cpu1)

Thanks.

-- 
tejun