linux-kernel - Re: [PATCH 3/6] x86: simplify _acpi_map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 02 Sep 2013 10:13:54 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Tony Luck <tony.luck@...el.com>,
	Ingo Molnar <mingo@...hat.com>, linux-acpi@...r.kernel.org,
	x86@...nel.org, linux-ia64@...r.kernel.org,
	linux-kernel@...r.kernel.org, patches@...aro.org,
	linaro-kernel@...ts.linaro.org, linaro-acpi@...ts.linaro.org,
	Jiang Liu <jiang.liu@...wei.com>
Subject: Re: [PATCH 3/6] x86: simplify _acpi_map_lsapic()

On 2013-9-1 4:14, Rafael J. Wysocki wrote:
> On Saturday, August 31, 2013 06:15:58 PM Hanjun Guo wrote:
[...]
>> +static int acpi_register_lapic(int id, u8 enabled)
>>  {
>>  	unsigned int ver = 0;
>>  
>>  	if (id >= (MAX_LOCAL_APIC-1)) {
>>  		printk(KERN_INFO PREFIX "skipped apicid that is too big\n");
>> -		return;
>> +		return -1;
>>  	}
>>  
>>  	if (!enabled) {
>>  		++disabled_cpus;
>> -		return;
>> +		return -1;
>>  	}
> 
> If this returned -EINVAL instead of just -1, ->
> 
>>  
>>  	if (boot_cpu_physical_apicid != -1U)
>>  		ver = apic_version[boot_cpu_physical_apicid];
>>  
>> -	generic_processor_info(id, ver);
>> +	return generic_processor_info(id, ver);
> 
> -> and this too (on errors), -->
> 
>>  }
>>  
>>  static int __init
>> @@ -622,44 +629,19 @@ static void acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
>>  
>>  static int _acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu)
>>  {
>> -	cpumask_var_t tmp_map, new_map;
>>  	int cpu;
>> -	int retval = -ENOMEM;
>> -
>> -	if (!alloc_cpumask_var(&tmp_map, GFP_KERNEL))
>> -		goto out;
>>  
>> -	if (!alloc_cpumask_var(&new_map, GFP_KERNEL))
>> -		goto free_tmp_map;
>> -
>> -	cpumask_copy(tmp_map, cpu_present_mask);
>> -	acpi_register_lapic(physid, ACPI_MADT_ENABLED);
>> -
>> -	/*
>> -	 * If acpi_register_lapic successfully generates a new logical cpu
>> -	 * number, then the following will get us exactly what was mapped
>> -	 */
>> -	cpumask_andnot(new_map, cpu_present_mask, tmp_map);
>> -	if (cpumask_empty(new_map)) {
>> +	cpu = acpi_register_lapic(physid, ACPI_MADT_ENABLED);
>> +	if (cpu == -1) {
>>  		printk ("Unable to map lapic to logical cpu number\n");
>> -		retval = -EINVAL;
>> -		goto free_new_map;
>> +		return -EINVAL;
>>  	}
> 
> --> then this could be
> 
> 	if (cpu < 0) {
> 		...
> 		return cpu;
> 	}

Thanks for the suggestion, and I find out some grammar mistake in the
changelog too, I will update this patch set and send out soon.

Thanks
Hanjun

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/