* [PATCH] mpparse.c:231: warning: comparison is always false
@ 2006-09-13 6:50 Jarek Poplawski
2006-09-13 16:42 ` Dave Jones
[not found] ` <20060914181754.bd963f6d.akpm@osdl.org>
0 siblings, 2 replies; 14+ messages in thread
From: Jarek Poplawski @ 2006-09-13 6:50 UTC (permalink / raw)
To: linux-kernel
Hello,
Probably after 2.6.18-rc6-git1 there is this cc warning:
"arch/i386/kernel/mpparse.c:231: warning: comparison is
always false due to limited range of data type".
Maybe this patch will be helpful.
Jarek P.
diff -Nurp linux-2.6.18-rc6-git4-/arch/i386/kernel/mpparse.c linux-2.6.18-rc6-git4/arch/i386/kernel/mpparse.c
--- linux-2.6.18-rc6-git4-/arch/i386/kernel/mpparse.c 2006-09-13 00:01:00.000000000 +0200
+++ linux-2.6.18-rc6-git4/arch/i386/kernel/mpparse.c 2006-09-13 00:01:00.000000000 +0200
@@ -228,12 +228,14 @@ static void __init MP_bus_info (struct m
mpc_oem_bus_info(m, str, translation_table[mpc_record]);
+#if 0xFF >= MAX_MP_BUSSES
if (m->mpc_busid >= MAX_MP_BUSSES) {
printk(KERN_WARNING "MP table busid value (%d) for bustype %s "
" is too large, max. supported is %d\n",
m->mpc_busid, str, MAX_MP_BUSSES - 1);
return;
}
+#endif
if (strncmp(str, BUSTYPE_ISA, sizeof(BUSTYPE_ISA)-1) == 0) {
mp_bus_id_to_type[m->mpc_busid] = MP_BUS_ISA;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-13 6:50 [PATCH] mpparse.c:231: warning: comparison is always false Jarek Poplawski
@ 2006-09-13 16:42 ` Dave Jones
2006-09-14 5:36 ` Jarek Poplawski
2006-09-14 10:12 ` Andi Kleen
[not found] ` <20060914181754.bd963f6d.akpm@osdl.org>
1 sibling, 2 replies; 14+ messages in thread
From: Dave Jones @ 2006-09-13 16:42 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: linux-kernel, ak
On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
> Probably after 2.6.18-rc6-git1 there is this cc warning:
> "arch/i386/kernel/mpparse.c:231: warning: comparison is
> always false due to limited range of data type".
> Maybe this patch will be helpful.
>
> diff -Nurp linux-2.6.18-rc6-git4-/arch/i386/kernel/mpparse.c linux-2.6.18-rc6-git4/arch/i386/kernel/mpparse.c
> --- linux-2.6.18-rc6-git4-/arch/i386/kernel/mpparse.c 2006-09-13 00:01:00.000000000 +0200
> +++ linux-2.6.18-rc6-git4/arch/i386/kernel/mpparse.c 2006-09-13 00:01:00.000000000 +0200
> @@ -228,12 +228,14 @@ static void __init MP_bus_info (struct m
>
> mpc_oem_bus_info(m, str, translation_table[mpc_record]);
>
> +#if 0xFF >= MAX_MP_BUSSES
> if (m->mpc_busid >= MAX_MP_BUSSES) {
> printk(KERN_WARNING "MP table busid value (%d) for bustype %s "
> " is too large, max. supported is %d\n",
> m->mpc_busid, str, MAX_MP_BUSSES - 1);
> return;
> }
> +#endif
mpc_busid is a uchar. I don't see how this can ever be > 0xff, yet
mach-summit and mach-generic have MAX_MP_BUSSES set to 260.
I don't see how this can possibly work.
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-13 16:42 ` Dave Jones
@ 2006-09-14 5:36 ` Jarek Poplawski
2006-09-14 6:35 ` Dave Jones
2006-09-14 10:12 ` Andi Kleen
1 sibling, 1 reply; 14+ messages in thread
From: Jarek Poplawski @ 2006-09-14 5:36 UTC (permalink / raw)
To: Dave Jones; +Cc: linux-kernel
On Wed, Sep 13, 2006 at 12:42:51PM -0400, Dave Jones wrote:
> On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
...
> > +#if 0xFF >= MAX_MP_BUSSES
> > if (m->mpc_busid >= MAX_MP_BUSSES) {
> > printk(KERN_WARNING "MP table busid value (%d) for bustype %s "
> > " is too large, max. supported is %d\n",
> > m->mpc_busid, str, MAX_MP_BUSSES - 1);
> > return;
> > }
> > +#endif
>
> mpc_busid is a uchar. I don't see how this can ever be > 0xff, yet
> mach-summit and mach-generic have MAX_MP_BUSSES set to 260.
>
> I don't see how this can possibly work.
>
> Dave
>
0xFF >= 260 is false so the block is not compiled and
the warning is gone (+ several bytes of useless code).
Jarek P.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-14 5:36 ` Jarek Poplawski
@ 2006-09-14 6:35 ` Dave Jones
2006-09-14 7:36 ` Jarek Poplawski
0 siblings, 1 reply; 14+ messages in thread
From: Dave Jones @ 2006-09-14 6:35 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: linux-kernel
On Thu, Sep 14, 2006 at 07:36:47AM +0200, Jarek Poplawski wrote:
> On Wed, Sep 13, 2006 at 12:42:51PM -0400, Dave Jones wrote:
> > On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
> ...
> > > +#if 0xFF >= MAX_MP_BUSSES
> > > if (m->mpc_busid >= MAX_MP_BUSSES) {
> > > printk(KERN_WARNING "MP table busid value (%d) for bustype %s "
> > > " is too large, max. supported is %d\n",
> > > m->mpc_busid, str, MAX_MP_BUSSES - 1);
> > > return;
> > > }
> > > +#endif
> >
> > mpc_busid is a uchar. I don't see how this can ever be > 0xff, yet
> > mach-summit and mach-generic have MAX_MP_BUSSES set to 260.
> >
> > I don't see how this can possibly work.
>
> 0xFF >= 260 is false so the block is not compiled and
> the warning is gone (+ several bytes of useless code).
Yes, I understand your patch. What I don't understand is why summit/generic
have this set so high in the first place.
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-14 6:35 ` Dave Jones
@ 2006-09-14 7:36 ` Jarek Poplawski
0 siblings, 0 replies; 14+ messages in thread
From: Jarek Poplawski @ 2006-09-14 7:36 UTC (permalink / raw)
To: Dave Jones, linux-kernel
On Thu, Sep 14, 2006 at 02:35:42AM -0400, Dave Jones wrote:
...
> Yes, I understand your patch. What I don't understand is why summit/generic
> have this set so high in the first place.
Sorry for misunderstanding your understanding...
I simply tried to understand another question (how this warning
could be tolerated for all those gits?).
Jarek P.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-13 16:42 ` Dave Jones
2006-09-14 5:36 ` Jarek Poplawski
@ 2006-09-14 10:12 ` Andi Kleen
2006-09-14 16:08 ` Dave Jones
1 sibling, 1 reply; 14+ messages in thread
From: Andi Kleen @ 2006-09-14 10:12 UTC (permalink / raw)
To: Dave Jones; +Cc: Jarek Poplawski, linux-kernel
On Wednesday 13 September 2006 18:42, Dave Jones wrote:
> On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
> > Probably after 2.6.18-rc6-git1 there is this cc warning:
> > "arch/i386/kernel/mpparse.c:231: warning: comparison is
> > always false due to limited range of data type".
> > Maybe this patch will be helpful.
> >
It's already fixed.
-Andi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-14 10:12 ` Andi Kleen
@ 2006-09-14 16:08 ` Dave Jones
2006-09-15 6:43 ` Andi Kleen
0 siblings, 1 reply; 14+ messages in thread
From: Dave Jones @ 2006-09-14 16:08 UTC (permalink / raw)
To: Andi Kleen; +Cc: Jarek Poplawski, linux-kernel
On Thu, Sep 14, 2006 at 12:12:50PM +0200, Andi Kleen wrote:
> On Wednesday 13 September 2006 18:42, Dave Jones wrote:
> > On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
> > > Probably after 2.6.18-rc6-git1 there is this cc warning:
> > > "arch/i386/kernel/mpparse.c:231: warning: comparison is
> > > always false due to limited range of data type".
> > > Maybe this patch will be helpful.
> > >
>
> It's already fixed.
>
> -Andi
Still looks busted to me in 2.6.18rc7
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-14 16:08 ` Dave Jones
@ 2006-09-15 6:43 ` Andi Kleen
0 siblings, 0 replies; 14+ messages in thread
From: Andi Kleen @ 2006-09-15 6:43 UTC (permalink / raw)
To: Dave Jones; +Cc: Jarek Poplawski, linux-kernel
On Thursday 14 September 2006 18:08, Dave Jones wrote:
> On Thu, Sep 14, 2006 at 12:12:50PM +0200, Andi Kleen wrote:
> > On Wednesday 13 September 2006 18:42, Dave Jones wrote:
> > > On Wed, Sep 13, 2006 at 08:50:10AM +0200, Jarek Poplawski wrote:
> > > > Probably after 2.6.18-rc6-git1 there is this cc warning:
> > > > "arch/i386/kernel/mpparse.c:231: warning: comparison is
> > > > always false due to limited range of data type".
> > > > Maybe this patch will be helpful.
> >
> > It's already fixed.
> >
> > -Andi
>
> Still looks busted to me in 2.6.18rc7
Fixed in the queued for 2.6.19 tree. I didn't consider it important enough
to move up.
-Andi
^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20060914181754.bd963f6d.akpm@osdl.org>]
* Re: [PATCH] mpparse.c:231: warning: comparison is always false
[not found] ` <20060914181754.bd963f6d.akpm@osdl.org>
@ 2006-09-15 8:11 ` Jarek Poplawski
2006-09-15 8:23 ` Andrew Morton
0 siblings, 1 reply; 14+ messages in thread
From: Jarek Poplawski @ 2006-09-15 8:11 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andi Kleen, Dave Jones, linux-kernel
On Thu, Sep 14, 2006 at 06:17:54PM -0700, Andrew Morton wrote:
> On Wed, 13 Sep 2006 08:50:10 +0200
> Jarek Poplawski <jarkao2@o2.pl> wrote:
>
> > Hello,
> >
> > Probably after 2.6.18-rc6-git1 there is this cc warning:
> > "arch/i386/kernel/mpparse.c:231: warning: comparison is
> > always false due to limited range of data type".
> > Maybe this patch will be helpful.
> >
>
> Thanks. Andi has already queued a similar patch.
>
> Andi, you might as well scoot that upstream, otherwise we'll get lots of
> emails about it.
...
> > +#if 0xFF >= MAX_MP_BUSSES
> > if (m->mpc_busid >= MAX_MP_BUSSES) {
As a matter of fact today I think my patch is wrong.
I don't know how Andi has fixed it, but after rethinking
the question of Dave Jones I see it's fixing the result
instead of the source of a problem (char or not char).
So it's more serious problem just for really serious guys
like you.
Thanks for response,
Jarek P.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-15 8:11 ` Jarek Poplawski
@ 2006-09-15 8:23 ` Andrew Morton
2006-09-15 9:08 ` Jarek Poplawski
2006-09-15 15:23 ` Dave Jones
0 siblings, 2 replies; 14+ messages in thread
From: Andrew Morton @ 2006-09-15 8:23 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Andi Kleen, Dave Jones, linux-kernel
On Fri, 15 Sep 2006 10:11:23 +0200
Jarek Poplawski <jarkao2@o2.pl> wrote:
> On Thu, Sep 14, 2006 at 06:17:54PM -0700, Andrew Morton wrote:
> > On Wed, 13 Sep 2006 08:50:10 +0200
> > Jarek Poplawski <jarkao2@o2.pl> wrote:
> >
> > > Hello,
> > >
> > > Probably after 2.6.18-rc6-git1 there is this cc warning:
> > > "arch/i386/kernel/mpparse.c:231: warning: comparison is
> > > always false due to limited range of data type".
> > > Maybe this patch will be helpful.
> > >
> >
> > Thanks. Andi has already queued a similar patch.
> >
> > Andi, you might as well scoot that upstream, otherwise we'll get lots of
> > emails about it.
> ...
> > > +#if 0xFF >= MAX_MP_BUSSES
> > > if (m->mpc_busid >= MAX_MP_BUSSES) {
>
> As a matter of fact today I think my patch is wrong.
No, I think it's OK. Well, you had an off-by-one...
> I don't know how Andi has fixed it,
Same thing. (He has `#if MAX_MP_BUSSES < 256').
> but after rethinking
> the question of Dave Jones I see it's fixing the result
> instead of the source of a problem (char or not char).
The mpc_busid field is set to eight-bits by BIOS; there's nothing we can do
about that...
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-15 8:23 ` Andrew Morton
@ 2006-09-15 9:08 ` Jarek Poplawski
2006-09-15 15:23 ` Dave Jones
1 sibling, 0 replies; 14+ messages in thread
From: Jarek Poplawski @ 2006-09-15 9:08 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andi Kleen, Dave Jones, linux-kernel
On Fri, Sep 15, 2006 at 01:23:02AM -0700, Andrew Morton wrote:
> On Fri, 15 Sep 2006 10:11:23 +0200
> Jarek Poplawski <jarkao2@o2.pl> wrote:
> > As a matter of fact today I think my patch is wrong.
...
> No, I think it's OK. Well, you had an off-by-one...
just like the source:
> +#if 0xFF >= MAX_MP_BUSSES
> if (m->mpc_busid >= MAX_MP_BUSSES) {
...
> > but after rethinking
> > the question of Dave Jones I see it's fixing the result
> > instead of the source of a problem (char or not char).
>
> The mpc_busid field is set to eight-bits by BIOS; there's nothing we can do
> about that...
So IMHO maybe: if we can know this only by BIOS it should be
eight-bits - if there is another way to get this: shouln't
you add second constant? Now it's unlogical for me (and it
induces this strange #ifs in the code instead of headers).
Jarek P.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-15 8:23 ` Andrew Morton
2006-09-15 9:08 ` Jarek Poplawski
@ 2006-09-15 15:23 ` Dave Jones
2006-09-15 19:33 ` Andrew Morton
1 sibling, 1 reply; 14+ messages in thread
From: Dave Jones @ 2006-09-15 15:23 UTC (permalink / raw)
To: Andrew Morton; +Cc: Jarek Poplawski, Andi Kleen, linux-kernel
On Fri, Sep 15, 2006 at 01:23:02AM -0700, Andrew Morton wrote:
> > > Thanks. Andi has already queued a similar patch.
> > >
> > > Andi, you might as well scoot that upstream, otherwise we'll get lots of
> > > emails about it.
> > ...
> > > > +#if 0xFF >= MAX_MP_BUSSES
> > > > if (m->mpc_busid >= MAX_MP_BUSSES) {
> > I don't know how Andi has fixed it,
> Same thing. (He has `#if MAX_MP_BUSSES < 256').
How can this be the right the right thing to do ?
It should *never* be >=256. mach-summit/mach-generic need fixing
to be 255, not this ridiculous band-aid. Where did 260 come from anyway?
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-15 15:23 ` Dave Jones
@ 2006-09-15 19:33 ` Andrew Morton
2006-09-16 6:19 ` Andi Kleen
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2006-09-15 19:33 UTC (permalink / raw)
To: Dave Jones; +Cc: Jarek Poplawski, Andi Kleen, linux-kernel
On Fri, 15 Sep 2006 11:23:49 -0400
Dave Jones <davej@redhat.com> wrote:
> On Fri, Sep 15, 2006 at 01:23:02AM -0700, Andrew Morton wrote:
>
> > > > Thanks. Andi has already queued a similar patch.
> > > >
> > > > Andi, you might as well scoot that upstream, otherwise we'll get lots of
> > > > emails about it.
> > > ...
> > > > > +#if 0xFF >= MAX_MP_BUSSES
> > > > > if (m->mpc_busid >= MAX_MP_BUSSES) {
> > > I don't know how Andi has fixed it,
> > Same thing. (He has `#if MAX_MP_BUSSES < 256').
>
> How can this be the right the right thing to do ?
> It should *never* be >=256. mach-summit/mach-generic need fixing
> to be 255, not this ridiculous band-aid. Where did 260 come from anyway?
>
commit f0bacaf5cec4e677a00b5ab06d95664d03a30f7a
Author: akpm <akpm>
Date: Mon Apr 12 20:06:32 2004 +0000
[PATCH] summmit: increase MAX_MP_BUSSES
From: James Cleverdon <jamesclv@us.ibm.com>
Bump up MAX_MP_BUSSES for summit/generic subarch to cope with big IBM x440
systems.
BKrev: 407af6c8l8rvwRmEU-JHTS98MurIZA
diff --git a/include/asm-i386/mach-generic/mach_mpspec.h b/include/asm-i386/mach-generic/mach_mpspec.h
index ef10cd2..fbb6a40 100644
--- a/include/asm-i386/mach-generic/mach_mpspec.h
+++ b/include/asm-i386/mach-generic/mach_mpspec.h
@@ -8,6 +8,8 @@ #define MAX_APICS 256
#define MAX_IRQ_SOURCES 256
-#define MAX_MP_BUSSES 32
+/* Summit or generic (i.e. installer) kernels need lots of bus entries. */
+/* Maximum 256 PCI busses, plus 1 ISA bus in each of 4 cabinets. */
+#define MAX_MP_BUSSES 260
#endif /* __ASM_MACH_MPSPEC_H */
diff --git a/include/asm-i386/mach-summit/mach_mpspec.h b/include/asm-i386/mach-summit/mach_mpspec.h
index ef10cd2..bc8f717 100644
--- a/include/asm-i386/mach-summit/mach_mpspec.h
+++ b/include/asm-i386/mach-summit/mach_mpspec.h
@@ -8,6 +8,7 @@ #define MAX_APICS 256
#define MAX_IRQ_SOURCES 256
-#define MAX_MP_BUSSES 32
+/* Maximum 256 PCI busses, plus 1 ISA bus in each of 4 cabinets. */
+#define MAX_MP_BUSSES 260
#endif /* __ASM_MACH_MPSPEC_H */
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] mpparse.c:231: warning: comparison is always false
2006-09-15 19:33 ` Andrew Morton
@ 2006-09-16 6:19 ` Andi Kleen
0 siblings, 0 replies; 14+ messages in thread
From: Andi Kleen @ 2006-09-16 6:19 UTC (permalink / raw)
To: Andrew Morton; +Cc: Dave Jones, Jarek Poplawski, linux-kernel
On Fri, Sep 15, 2006 at 12:33:40PM -0700, Andrew Morton wrote:
> On Fri, 15 Sep 2006 11:23:49 -0400
> Dave Jones <davej@redhat.com> wrote:
>
> > On Fri, Sep 15, 2006 at 01:23:02AM -0700, Andrew Morton wrote:
> >
> > > > > Thanks. Andi has already queued a similar patch.
> > > > >
> > > > > Andi, you might as well scoot that upstream, otherwise we'll get lots of
> > > > > emails about it.
> > > > ...
> > > > > > +#if 0xFF >= MAX_MP_BUSSES
> > > > > > if (m->mpc_busid >= MAX_MP_BUSSES) {
> > > > I don't know how Andi has fixed it,
> > > Same thing. (He has `#if MAX_MP_BUSSES < 256').
> >
> > How can this be the right the right thing to do ?
> > It should *never* be >=256. mach-summit/mach-generic need fixing
> > to be 255, not this ridiculous band-aid. Where did 260 come from anyway?
> >
>
> commit f0bacaf5cec4e677a00b5ab06d95664d03a30f7a
> Author: akpm <akpm>
> Date: Mon Apr 12 20:06:32 2004 +0000
>
> [PATCH] summmit: increase MAX_MP_BUSSES
>
> From: James Cleverdon <jamesclv@us.ibm.com>
>
> Bump up MAX_MP_BUSSES for summit/generic subarch to cope with big IBM x440
> systems.
The 260 is because ACPI can create larger busses and the summit
boxes only run with ACPI anyways.
-Andi
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2006-09-16 6:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-13 6:50 [PATCH] mpparse.c:231: warning: comparison is always false Jarek Poplawski
2006-09-13 16:42 ` Dave Jones
2006-09-14 5:36 ` Jarek Poplawski
2006-09-14 6:35 ` Dave Jones
2006-09-14 7:36 ` Jarek Poplawski
2006-09-14 10:12 ` Andi Kleen
2006-09-14 16:08 ` Dave Jones
2006-09-15 6:43 ` Andi Kleen
[not found] ` <20060914181754.bd963f6d.akpm@osdl.org>
2006-09-15 8:11 ` Jarek Poplawski
2006-09-15 8:23 ` Andrew Morton
2006-09-15 9:08 ` Jarek Poplawski
2006-09-15 15:23 ` Dave Jones
2006-09-15 19:33 ` Andrew Morton
2006-09-16 6:19 ` Andi Kleen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox