All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hanjun Guo <hanjun.guo@linaro.org>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Tony Luck <tony.luck@intel.com>, Ingo Molnar <mingo@redhat.com>,
	linux-acpi@vger.kernel.org, x86@kernel.org,
	linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@linaro.org, linaro-kernel@lists.linaro.org,
	linaro-acpi@lists.linaro.org, Jiang Liu <jiang.liu@huawei.com>
Subject: Re: [PATCH 3/6] x86: simplify _acpi_map_lsapic()
Date: Mon, 02 Sep 2013 10:13:54 +0800	[thread overview]
Message-ID: <5223F462.7090500@linaro.org> (raw)
In-Reply-To: <8575859.HxOyLiBtGF@vostro.rjw.lan>

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


WARNING: multiple messages have this Message-ID (diff)
From: Hanjun Guo <hanjun.guo@linaro.org>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Tony Luck <tony.luck@intel.com>, Ingo Molnar <mingo@redhat.com>,
	linux-acpi@vger.kernel.org, x86@kernel.org,
	linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@linaro.org, linaro-kernel@lists.linaro.org,
	linaro-acpi@lists.linaro.org, Jiang Liu <jiang.liu@huawei.com>
Subject: Re: [PATCH 3/6] x86: simplify _acpi_map_lsapic()
Date: Mon, 02 Sep 2013 02:13:54 +0000	[thread overview]
Message-ID: <5223F462.7090500@linaro.org> (raw)
In-Reply-To: <8575859.HxOyLiBtGF@vostro.rjw.lan>

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


  reply	other threads:[~2013-09-02  2:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-31 10:15 [PATCH 1/6] ACPI / processor: Introduce apic_id in struct processor to save parsed APIC id Hanjun Guo
2013-08-31 10:15 ` Hanjun Guo
2013-08-31 10:15 ` [PATCH 2/6] ACPI / processor: use apic_id and remove duplicated _MAT evaluation Hanjun Guo
2013-08-31 10:15   ` Hanjun Guo
2013-08-31 10:15 ` [PATCH 3/6] x86: simplify _acpi_map_lsapic() Hanjun Guo
2013-08-31 10:15   ` Hanjun Guo
2013-08-31 20:14   ` Rafael J. Wysocki
2013-08-31 20:14     ` Rafael J. Wysocki
2013-09-02  2:13     ` Hanjun Guo [this message]
2013-09-02  2:13       ` Hanjun Guo
2013-08-31 10:15 ` [PATCH 4/6] ACPI / processor: remove some dead code in acpi_processor_get_info() Hanjun Guo
2013-08-31 10:15   ` Hanjun Guo
2013-08-31 10:16 ` [PATCH 5/6] ACPI / processor: remove unnecessary if (!pr) check Hanjun Guo
2013-08-31 10:16   ` Hanjun Guo
2013-08-31 10:16 ` [PATCH 6/6] ACPI / processor: Remove outdated comments Hanjun Guo
2013-08-31 10:16   ` Hanjun Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5223F462.7090500@linaro.org \
    --to=hanjun.guo@linaro.org \
    --cc=jiang.liu@huawei.com \
    --cc=linaro-acpi@lists.linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=patches@linaro.org \
    --cc=rjw@sisk.pl \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.