Re: [rfc 01/45] ACPI: Avoid references to impossible processors.

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* clameter@sgi.com (clameter@sgi.com) wrote:
> ACPI uses NR_CPUS in various loops and in some it accesses per cpu
> data of processors that are not present(!) and that will never be present.
> The pointers to per cpu data are typically not initialized for processors
> that are not present. So we seem to be reading something here from offset 0
> in memory.
> 
> Make ACPI use nr_cpu_ids instead. That stops at the end of the possible
> processors.
> 
> Convert one loop to NR_CPUS to use the cpu_possible map instead.
> 

I'm just wondering how broken this is. Is there any assumption that
there is no holes in the online cpu map in this code ?

We can very well have :

0 off
1 on
2 on
3 on
...

> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> ---
>  drivers/acpi/processor_core.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/processor_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_core.c	2007-11-19 15:45:05.041140492 -0800
> +++ linux-2.6/drivers/acpi/processor_core.c	2007-11-19 15:48:22.513639920 -0800
> @@ -494,7 +494,7 @@ static int get_cpu_id(acpi_handle handle
>  	if (apic_id == -1)
>  		return apic_id;
>  
> -	for (i = 0; i < NR_CPUS; ++i) {
> +	for_each_possible_cpu(i) {
>  		if (cpu_physical_id(i) == apic_id)
>  			return i;
>  	}
> @@ -638,7 +638,7 @@ static int __cpuinit acpi_processor_star
>  		return 0;
>  	}
>  
> -	BUG_ON((pr->id >= NR_CPUS) || (pr->id < 0));
> +	BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
>  
>  	/*
>  	 * Buggy BIOS check
> @@ -771,7 +771,7 @@ static int acpi_processor_remove(struct 
>  
>  	pr = acpi_driver_data(device);
>  
> -	if (pr->id >= NR_CPUS) {
> +	if (pr->id >= nr_cpu_ids) {
>  		kfree(pr);
>  		return 0;
>  	}
> @@ -842,7 +842,7 @@ int acpi_processor_device_add(acpi_handl
>  	if (!pr)
>  		return -ENODEV;
>  
> -	if ((pr->id >= 0) && (pr->id < NR_CPUS)) {
> +	if ((pr->id >= 0) && (pr->id < nr_cpu_ids)) {
>  		kobject_uevent(&(*device)->dev.kobj, KOBJ_ONLINE);
>  	}
>  	return 0;
> @@ -880,13 +880,13 @@ acpi_processor_hotplug_notify(acpi_handl
>  			break;
>  		}
>  
> -		if (pr->id >= 0 && (pr->id < NR_CPUS)) {
> +		if (pr->id >= 0 && (pr->id < nr_cpu_ids)) {
>  			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
>  			break;
>  		}
>  
>  		result = acpi_processor_start(device);
> -		if ((!result) && ((pr->id >= 0) && (pr->id < NR_CPUS))) {
> +		if ((!result) && ((pr->id >= 0) && (pr->id < nr_cpu_ids))) {
>  			kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
>  		} else {
>  			printk(KERN_ERR PREFIX "Device [%s] failed to start\n",
> @@ -909,7 +909,7 @@ acpi_processor_hotplug_notify(acpi_handl
>  			return;
>  		}
>  
> -		if ((pr->id < NR_CPUS) && (cpu_present(pr->id)))
> +		if ((pr->id < nr_cpu_ids) && (cpu_present(pr->id)))
>  			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
>  		break;
>  	default:
> 
> -- 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/