* [PATCH 1/2] ACPI/tables: Correct the wrong count increasing
@ 2016-08-08 23:01 Baoquan He
2016-08-08 23:01 ` [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries Baoquan He
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Baoquan He @ 2016-08-08 23:01 UTC (permalink / raw)
To: linux-kernel; +Cc: Baoquan He, Rafael J. Wysocki, Len Brown, linux-acpi
The current code always increases the count in the 1st element of
array proc[].
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
drivers/acpi/tables.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 9f0ad6e..34d45bb 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -281,7 +281,7 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
proc[i].handler(entry, table_end))
return -EINVAL;
- proc->count++;
+ proc[i]->count++;
break;
}
if (i != proc_num)
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
2016-08-08 23:01 [PATCH 1/2] ACPI/tables: Correct the wrong count increasing Baoquan He
@ 2016-08-08 23:01 ` Baoquan He
2016-08-10 12:53 ` Ingo Molnar
2016-08-08 23:27 ` [PATCH 1/2] ACPI/tables: Correct the wrong count increasing kbuild test robot
2016-08-09 0:30 ` [PATCH v2 " Baoquan He
2 siblings, 1 reply; 11+ messages in thread
From: Baoquan He @ 2016-08-08 23:01 UTC (permalink / raw)
To: linux-kernel
Cc: Baoquan He, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm
It won't impact the result, we still should fix the code bug.
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: linux-pm@vger.kernel.org
---
arch/x86/kernel/acpi/boot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 90d84c3..2b25d3f 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1031,8 +1031,8 @@ static int __init acpi_parse_madt_lapic_entries(void)
return ret;
}
- x2count = madt_proc[0].count;
- count = madt_proc[1].count;
+ count = madt_proc[0].count;
+ x2count = madt_proc[1].count;
}
if (!count && !x2count) {
printk(KERN_ERR PREFIX "No LAPIC entries present\n");
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI/tables: Correct the wrong count increasing
2016-08-08 23:01 [PATCH 1/2] ACPI/tables: Correct the wrong count increasing Baoquan He
2016-08-08 23:01 ` [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries Baoquan He
@ 2016-08-08 23:27 ` kbuild test robot
2016-08-09 0:28 ` Baoquan He
2016-08-09 0:30 ` [PATCH v2 " Baoquan He
2 siblings, 1 reply; 11+ messages in thread
From: kbuild test robot @ 2016-08-08 23:27 UTC (permalink / raw)
To: Baoquan He
Cc: kbuild-all, linux-kernel, Baoquan He, Rafael J. Wysocki,
Len Brown, linux-acpi
[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]
Hi Baoquan,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.8-rc1 next-20160808]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Baoquan-He/ACPI-tables-Correct-the-wrong-count-increasing/20160809-070345
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-x002-201632 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/acpi/tables.c: In function 'acpi_parse_entries_array':
>> drivers/acpi/tables.c:284:11: error: invalid type argument of '->' (have 'struct acpi_subtable_proc')
proc[i]->count++;
^~
vim +284 drivers/acpi/tables.c
278 if (entry->type != proc[i].id)
279 continue;
280 if (!proc[i].handler ||
281 proc[i].handler(entry, table_end))
282 return -EINVAL;
283
> 284 proc[i]->count++;
285 break;
286 }
287 if (i != proc_num)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25801 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI/tables: Correct the wrong count increasing
2016-08-08 23:27 ` [PATCH 1/2] ACPI/tables: Correct the wrong count increasing kbuild test robot
@ 2016-08-09 0:28 ` Baoquan He
0 siblings, 0 replies; 11+ messages in thread
From: Baoquan He @ 2016-08-09 0:28 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, linux-kernel, Rafael J. Wysocki, Len Brown, linux-acpi
Sorry, I tested in another system, but forget updating code on my
laptop.
Will reply with a v2 post.
Sorry again about this mistake.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] ACPI/tables: Correct the wrong count increasing
2016-08-08 23:01 [PATCH 1/2] ACPI/tables: Correct the wrong count increasing Baoquan He
2016-08-08 23:01 ` [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries Baoquan He
2016-08-08 23:27 ` [PATCH 1/2] ACPI/tables: Correct the wrong count increasing kbuild test robot
@ 2016-08-09 0:30 ` Baoquan He
2016-08-16 2:26 ` Zheng, Lv
2 siblings, 1 reply; 11+ messages in thread
From: Baoquan He @ 2016-08-09 0:30 UTC (permalink / raw)
To: linux-kernel; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi
The current code always increases the count in the 1st element of
array proc[].
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
v1->v2:
V1 is a wrong post because I didn't update the tested code to my
local laptop. Repost with a correct v2.
drivers/acpi/tables.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 9f0ad6e..34d45bb 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -281,7 +281,7 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
proc[i].handler(entry, table_end))
return -EINVAL;
- proc->count++;
+ proc[i].count++;
break;
}
if (i != proc_num)
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
2016-08-08 23:01 ` [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries Baoquan He
@ 2016-08-10 12:53 ` Ingo Molnar
2016-08-10 13:03 ` Baoquan He
0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-08-10 12:53 UTC (permalink / raw)
To: Baoquan He
Cc: linux-kernel, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm
* Baoquan He <bhe@redhat.com> wrote:
> It won't impact the result, we still should fix the code bug.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: linux-pm@vger.kernel.org
> ---
> arch/x86/kernel/acpi/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 90d84c3..2b25d3f 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -1031,8 +1031,8 @@ static int __init acpi_parse_madt_lapic_entries(void)
> return ret;
> }
>
> - x2count = madt_proc[0].count;
> - count = madt_proc[1].count;
> + count = madt_proc[0].count;
> + x2count = madt_proc[1].count;
> }
> if (!count && !x2count) {
> printk(KERN_ERR PREFIX "No LAPIC entries present\n");
Why does this bug have no effect?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
2016-08-10 12:53 ` Ingo Molnar
@ 2016-08-10 13:03 ` Baoquan He
2016-08-11 8:46 ` Ingo Molnar
0 siblings, 1 reply; 11+ messages in thread
From: Baoquan He @ 2016-08-10 13:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm
On 08/10/16 at 02:53pm, Ingo Molnar wrote:
>
> * Baoquan He <bhe@redhat.com> wrote:
>
> > It won't impact the result, we still should fix the code bug.
> >
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: x86@kernel.org
> > Cc: linux-pm@vger.kernel.org
> > ---
> > arch/x86/kernel/acpi/boot.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> > index 90d84c3..2b25d3f 100644
> > --- a/arch/x86/kernel/acpi/boot.c
> > +++ b/arch/x86/kernel/acpi/boot.c
> > @@ -1031,8 +1031,8 @@ static int __init acpi_parse_madt_lapic_entries(void)
> > return ret;
> > }
> >
> > - x2count = madt_proc[0].count;
> > - count = madt_proc[1].count;
> > + count = madt_proc[0].count;
> > + x2count = madt_proc[1].count;
> > }
> > if (!count && !x2count) {
~~~~~~~~~~~~~~~~~~~~~
I mean here the value checking won't be impacted by the wrong
assignment.
> > printk(KERN_ERR PREFIX "No LAPIC entries present\n");
>
> Why does this bug have no effect?
>
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
2016-08-10 13:03 ` Baoquan He
@ 2016-08-11 8:46 ` Ingo Molnar
2016-08-11 14:20 ` Baoquan He
0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-08-11 8:46 UTC (permalink / raw)
To: Baoquan He
Cc: linux-kernel, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm
* Baoquan He <bhe@redhat.com> wrote:
> On 08/10/16 at 02:53pm, Ingo Molnar wrote:
> >
> > * Baoquan He <bhe@redhat.com> wrote:
> >
> > > It won't impact the result, we still should fix the code bug.
> > >
> > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Cc: Len Brown <len.brown@intel.com>
> > > Cc: Pavel Machek <pavel@ucw.cz>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > > Cc: x86@kernel.org
> > > Cc: linux-pm@vger.kernel.org
> > > ---
> > > arch/x86/kernel/acpi/boot.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> > > index 90d84c3..2b25d3f 100644
> > > --- a/arch/x86/kernel/acpi/boot.c
> > > +++ b/arch/x86/kernel/acpi/boot.c
> > > @@ -1031,8 +1031,8 @@ static int __init acpi_parse_madt_lapic_entries(void)
> > > return ret;
> > > }
> > >
> > > - x2count = madt_proc[0].count;
> > > - count = madt_proc[1].count;
> > > + count = madt_proc[0].count;
> > > + x2count = madt_proc[1].count;
> > > }
> > > if (!count && !x2count) {
> ~~~~~~~~~~~~~~~~~~~~~
> I mean here the value checking won't be impacted by the wrong
> assignment.
Indeed!
Mind putting that into the changelog? Something like:
"By pure accident the bug makes no functional difference, because the only
expression where we are using these values is (!count && !x2count), in which
the variables are interchangeable, but it makes sense to fix the bug
nevertheless."
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
2016-08-11 8:46 ` Ingo Molnar
@ 2016-08-11 14:20 ` Baoquan He
0 siblings, 0 replies; 11+ messages in thread
From: Baoquan He @ 2016-08-11 14:20 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm
On 08/11/16 at 10:46am, Ingo Molnar wrote:
> > > > - count = madt_proc[1].count;
> > > > + count = madt_proc[0].count;
> > > > + x2count = madt_proc[1].count;
> > > > }
> > > > if (!count && !x2count) {
> > ~~~~~~~~~~~~~~~~~~~~~
> > I mean here the value checking won't be impacted by the wrong
> > assignment.
>
> Indeed!
>
> Mind putting that into the changelog? Something like:
>
> "By pure accident the bug makes no functional difference, because the only
> expression where we are using these values is (!count && !x2count), in which
> the variables are interchangeable, but it makes sense to fix the bug
> nevertheless."
Sure, this is much clearer. Will use this and repost. Thanks a lot, will
try to do each post using patch log like this.
Thanks
Baoquan
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 1/2] ACPI/tables: Correct the wrong count increasing
2016-08-09 0:30 ` [PATCH v2 " Baoquan He
@ 2016-08-16 2:26 ` Zheng, Lv
2016-08-16 8:21 ` Baoquan He
0 siblings, 1 reply; 11+ messages in thread
From: Zheng, Lv @ 2016-08-16 2:26 UTC (permalink / raw)
To: Baoquan He, linux-kernel; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi
Hi,
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Baoquan
> He
> Subject: [PATCH v2 1/2] ACPI/tables: Correct the wrong count increasing
>
> The current code always increases the count in the 1st element of
> array proc[].
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>
> v1->v2:
> V1 is a wrong post because I didn't update the tested code to my
> local laptop. Repost with a correct v2.
>
> drivers/acpi/tables.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 9f0ad6e..34d45bb 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -281,7 +281,7 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
> proc[i].handler(entry, table_end))
> return -EINVAL;
>
> - proc->count++;
> + proc[i].count++;
Do we have code using acpi_subtable_proce.count?
I think the answer is yes because of:
[Patch] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
So why don't you put these 2 patches together into a single series?
And help to validate if there are problems in other acpi_subtable_proce.count users.
Thanks
Lv
> break;
> }
> if (i != proc_num)
> --
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] ACPI/tables: Correct the wrong count increasing
2016-08-16 2:26 ` Zheng, Lv
@ 2016-08-16 8:21 ` Baoquan He
0 siblings, 0 replies; 11+ messages in thread
From: Baoquan He @ 2016-08-16 8:21 UTC (permalink / raw)
To: Zheng, Lv; +Cc: linux-kernel, Rafael J. Wysocki, Len Brown, linux-acpi
On 08/16/16 at 02:26am, Zheng, Lv wrote:
> Hi,
>
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Baoquan
> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> > index 9f0ad6e..34d45bb 100644
> > --- a/drivers/acpi/tables.c
> > +++ b/drivers/acpi/tables.c
> > @@ -281,7 +281,7 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
> > proc[i].handler(entry, table_end))
> > return -EINVAL;
> >
> > - proc->count++;
> > + proc[i].count++;
>
> Do we have code using acpi_subtable_proce.count?
> I think the answer is yes because of:
> [Patch] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries
>
> So why don't you put these 2 patches together into a single series?
> And help to validate if there are problems in other acpi_subtable_proce.count users.
Thanks for comments. I hesitated to put them into one patch or two
patches when I post. Later I decided to post them in two patches because
they are in two components, one is x86, the other is ACPI. And though
very simple fix I worry they can't be described well in one patch log.
Anyway, change related to patch 1/2 had been included in Al Stone's
patchset posted earlier. So this one has to be NACKed.
>
> Thanks
> Lv
>
> > break;
> > }
> > if (i != proc_num)
> > --
> > 2.5.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-08-16 8:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 23:01 [PATCH 1/2] ACPI/tables: Correct the wrong count increasing Baoquan He
2016-08-08 23:01 ` [PATCH 2/2] x86, ACPI: Fix the wrong assignment when Handle apic/x2apic entries Baoquan He
2016-08-10 12:53 ` Ingo Molnar
2016-08-10 13:03 ` Baoquan He
2016-08-11 8:46 ` Ingo Molnar
2016-08-11 14:20 ` Baoquan He
2016-08-08 23:27 ` [PATCH 1/2] ACPI/tables: Correct the wrong count increasing kbuild test robot
2016-08-09 0:28 ` Baoquan He
2016-08-09 0:30 ` [PATCH v2 " Baoquan He
2016-08-16 2:26 ` Zheng, Lv
2016-08-16 8:21 ` Baoquan He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).