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
next prev parent 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.