Patchwork [v2,10/16] Fix mdadm for powerpc64

login
register
mail settings
Submitter Matthew McClintock
Date Sept. 29, 2011, 6:52 p.m.
Message ID <1317322353-30258-1-git-send-email-msm@freescale.com>
Download mbox | patch
Permalink /patch/12427/
State New, archived
Headers show

Comments

Matthew McClintock - Sept. 29, 2011, 6:52 p.m.
This fixes build failures from build warnings on powerpc64

Signed-off-by: Matthew McClintock <msm@freescale.com>
---
 meta/recipes-extended/mdadm/mdadm_3.2.2.bb |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Richard Purdie - Sept. 30, 2011, 11:38 a.m.
On Thu, 2011-09-29 at 13:52 -0500, Matthew McClintock wrote:
> This fixes build failures from build warnings on powerpc64
> 
> Signed-off-by: Matthew McClintock <msm@freescale.com>
> ---
>  meta/recipes-extended/mdadm/mdadm_3.2.2.bb |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> index 5d29ae7..1c83f89 100644
> --- a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> +++ b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> @@ -8,7 +8,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=0636e73ff0215e8d672dc4c32c317bb3 \
>                      file://mdmon.c;beginline=4;endline=18;md5=af7d8444d9c4d3e5c7caac0d9d34039d \
>                      file://mdadm.h;beglinlne=4;endline=22;md5=462bc9936ac0d3da110191a3f9994161"
>  
> -PR = "r0"
> +PR = "r1"
>  
>  SRC_URI = "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.bz2"
>  
> @@ -16,6 +16,7 @@ SRC_URI[md5sum] = "12ee2fbf3beddb60601fb7a4c4905651"
>  SRC_URI[sha256sum] = "0d1a04e688b082bc88846e3f524abd50bc782b6ffc06123140f7d358c8f9b906"
>  
>  CFLAGS += "-fno-strict-aliasing"
> +EXTRA_OEMAKE_append_powerpc64 = "CXFLAGS=-Wno-error=format"

That isn't quite what I meant, that also overwrites anything existing in
CXFLAGS just in a different way. Also, I suspect you mean CXXFLAGS?

I'd meant something like:

CXXFLAGS_append_powerpc64 = " -Wno-error=format"

Cheers,

Richard
McClintock Matthew-B29882 - Sept. 30, 2011, 3:51 p.m.
On Fri, Sep 30, 2011 at 6:38 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> That isn't quite what I meant, that also overwrites anything existing in
> CXFLAGS just in a different way. Also, I suspect you mean CXXFLAGS?
>
> I'd meant something like:
>
> CXXFLAGS_append_powerpc64 = " -Wno-error=format"

In the mdadm Makefile it gives the following instructions:

# define "CXFLAGS" to give extra flags to CC.
# e.g.  make CXFLAGS=-O to optimise

So I was following that...

-M
Richard Purdie - Sept. 30, 2011, 4:29 p.m.
On Fri, 2011-09-30 at 15:51 +0000, McClintock Matthew-B29882 wrote:
> On Fri, Sep 30, 2011 at 6:38 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > That isn't quite what I meant, that also overwrites anything existing in
> > CXFLAGS just in a different way. Also, I suspect you mean CXXFLAGS?
> >
> > I'd meant something like:
> >
> > CXXFLAGS_append_powerpc64 = " -Wno-error=format"
> 
> In the mdadm Makefile it gives the following instructions:
> 
> # define "CXFLAGS" to give extra flags to CC.
> # e.g.  make CXFLAGS=-O to optimise
> 
> So I was following that...

Ok, normally we'd use CFLAGS for this but people love to be different
when writing makefiles :)

In that case your patch is fine...

Cheers,

Richard
Khem Raj - Oct. 3, 2011, 7:22 p.m.
On 9/29/2011 11:52 AM, Matthew McClintock wrote:
> This fixes build failures from build warnings on powerpc64
>
> Signed-off-by: Matthew McClintock<msm@freescale.com>
> ---
>   meta/recipes-extended/mdadm/mdadm_3.2.2.bb |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> index 5d29ae7..1c83f89 100644
> --- a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> +++ b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
> @@ -8,7 +8,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=0636e73ff0215e8d672dc4c32c317bb3 \
>                       file://mdmon.c;beginline=4;endline=18;md5=af7d8444d9c4d3e5c7caac0d9d34039d \
>                       file://mdadm.h;beglinlne=4;endline=22;md5=462bc9936ac0d3da110191a3f9994161"
>
> -PR = "r0"
> +PR = "r1"
>
>   SRC_URI = "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.bz2"
>
> @@ -16,6 +16,7 @@ SRC_URI[md5sum] = "12ee2fbf3beddb60601fb7a4c4905651"
>   SRC_URI[sha256sum] = "0d1a04e688b082bc88846e3f524abd50bc782b6ffc06123140f7d358c8f9b906"
>
>   CFLAGS += "-fno-strict-aliasing"
> +EXTRA_OEMAKE_append_powerpc64 = "CXFLAGS=-Wno-error=format"
>
>   inherit autotools
>

what kind of errors do you get ? and I wonder why they dont show up on 
x86_64. It may be something to fix in the madadm code.
McClintock Matthew-B29882 - Oct. 3, 2011, 9:14 p.m.
On Mon, Oct 3, 2011 at 2:22 PM, Khem Raj <raj.khem@gmail.com> wrote:
>> -PR = "r0"
>> +PR = "r1"
>>
>>  SRC_URI =
>> "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.bz2"
>>
>> @@ -16,6 +16,7 @@ SRC_URI[md5sum] = "12ee2fbf3beddb60601fb7a4c4905651"
>>  SRC_URI[sha256sum] =
>> "0d1a04e688b082bc88846e3f524abd50bc782b6ffc06123140f7d358c8f9b906"
>>
>>  CFLAGS += "-fno-strict-aliasing"
>> +EXTRA_OEMAKE_append_powerpc64 = "CXFLAGS=-Wno-error=format"
>>
>>  inherit autotools
>>
>
> what kind of errors do you get ? and I wonder why they dont show up on
> x86_64. It may be something to fix in the madadm code.

I can't find a log of this anywhere, it's probably the default
compiler warnings for our platform are different?

You could try enabling those errors and see if your build fails:

EXTRA_OEMAKE_append_x86-64 = "CXFLAGS=-Werror=format"

-M
Khem Raj - Oct. 3, 2011, 9:17 p.m.
On 10/3/2011 2:14 PM, McClintock Matthew-B29882 wrote:
> On Mon, Oct 3, 2011 at 2:22 PM, Khem Raj<raj.khem@gmail.com>  wrote:
>>> -PR = "r0"
>>> +PR = "r1"
>>>
>>>   SRC_URI =
>>> "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.bz2"
>>>
>>> @@ -16,6 +16,7 @@ SRC_URI[md5sum] = "12ee2fbf3beddb60601fb7a4c4905651"
>>>   SRC_URI[sha256sum] =
>>> "0d1a04e688b082bc88846e3f524abd50bc782b6ffc06123140f7d358c8f9b906"
>>>
>>>   CFLAGS += "-fno-strict-aliasing"
>>> +EXTRA_OEMAKE_append_powerpc64 = "CXFLAGS=-Wno-error=format"
>>>
>>>   inherit autotools
>>>
>>
>> what kind of errors do you get ? and I wonder why they dont show up on
>> x86_64. It may be something to fix in the madadm code.
>
> I can't find a log of this anywhere, it's probably the default
> compiler warnings for our platform are different?

It would be handy to understand them since I dont see gcc 
differentiating formats between ppc64 and x86_64 which both are 64bit 
hosts. But obviously there is something missing.

>
> You could try enabling those errors and see if your build fails:

If I had a ppc64 build around

>
> EXTRA_OEMAKE_append_x86-64 = "CXFLAGS=-Werror=format"
>
> -M
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
McClintock Matthew-B29882 - Oct. 4, 2011, 9:52 p.m.
On Mon, Oct 3, 2011 at 4:17 PM, Khem Raj <raj.khem@gmail.com> wrote:
>> I can't find a log of this anywhere, it's probably the default
>> compiler warnings for our platform are different?
>
> It would be handy to understand them since I dont see gcc differentiating
> formats between ppc64 and x86_64 which both are 64bit hosts. But obviously
> there is something missing.

I finally got around to rerunning this without my patch and the errors
are below.

/.mdadm/failed-slots\" -DUSE_PTHREADS   -c -o mdmon.o mdmon.c
| powerpc64-fsl-linux-gcc -Wall -Werror -Wstrict-prototypes -Wextra
-Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\"
-DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\"
-DMAP_DIR=\"/dev/.mdadm\" -DMAP_FILE=\"map\"
-DMDMON_DIR=\"/dev/.mdadm\"
-DFAILED_SLOTS_DIR=\"/dev/.mdadm/failed-slots\" -DUSE_PTHREADS   -c -o
monitor.o monitor.c
| powerpc64-fsl-linux-gcc -Wall -Werror -Wstrict-prototypes -Wextra
-Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\"
-DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\"
-DMAP_DIR=\"/dev/.mdadm\" -DMAP_FILE=\"map\"
-DMDMON_DIR=\"/dev/.mdadm\"
-DFAILED_SLOTS_DIR=\"/dev/.mdadm/failed-slots\" -DUSE_PTHREADS   -c -o
managemon.o managemon.c
| super-intel.c: In function 'getinfo_super_imsm_volume':
| super-intel.c:2327:4: error: format '%llu' expects argument of type
'long long unsigned int', but argument 3 has type '__u64'
[-Werror=format]
| super-intel.c:2327:4: error: format '%llu' expects argument of type
'long long unsigned int', but argument 4 has type '__u64'
[-Werror=format]
| cc1: all warnings being treated as errors

-M
Richard Purdie - Oct. 5, 2011, 11:34 a.m.
On Tue, 2011-10-04 at 21:52 +0000, McClintock Matthew-B29882 wrote:
> On Mon, Oct 3, 2011 at 4:17 PM, Khem Raj <raj.khem@gmail.com> wrote:
> >> I can't find a log of this anywhere, it's probably the default
> >> compiler warnings for our platform are different?
> >
> > It would be handy to understand them since I dont see gcc differentiating
> > formats between ppc64 and x86_64 which both are 64bit hosts. But obviously
> > there is something missing.
> 
> I finally got around to rerunning this without my patch and the errors
> are below.

I dug into this a bit. To quote asm/types.h:

/*
 * This is here because we used to use l64 for 64bit powerpc
 * and we don't want to impact user mode with our change to ll64
 * in the kernel.
 */
#if defined(__powerpc64__) && !defined(__KERNEL__)
# include <asm-generic/int-l64.h>
#else
# include <asm-generic/int-ll64.h>
#endif

So ppc64 kernel space uses ll64 and userspace uses l64.

This means __u64 is a long for ppc64 but a long long for x86_64. The
format errors occurs rightly because a ull is being printed from a ul
variable. This will happen to work but its messy.

Cheers,

Richard
McClintock Matthew-B29882 - Oct. 5, 2011, 4:53 p.m.
On Wed, Oct 5, 2011 at 6:34 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> I dug into this a bit. To quote asm/types.h:
>
> /*
>  * This is here because we used to use l64 for 64bit powerpc
>  * and we don't want to impact user mode with our change to ll64
>  * in the kernel.
>  */
> #if defined(__powerpc64__) && !defined(__KERNEL__)
> # include <asm-generic/int-l64.h>
> #else
> # include <asm-generic/int-ll64.h>
> #endif
>
> So ppc64 kernel space uses ll64 and userspace uses l64.
>
> This means __u64 is a long for ppc64 but a long long for x86_64. The
> format errors occurs rightly because a ull is being printed from a ul
> variable. This will happen to work but its messy.

Sorry, I'm missing if you are suggesting a solution here... do I need
to take an alternate approach? The alternative is to patch the
printfs?

-M
Richard Purdie - Oct. 5, 2011, 5 p.m.
On Wed, 2011-10-05 at 16:53 +0000, McClintock Matthew-B29882 wrote:
> On Wed, Oct 5, 2011 at 6:34 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > I dug into this a bit. To quote asm/types.h:
> >
> > /*
> >  * This is here because we used to use l64 for 64bit powerpc
> >  * and we don't want to impact user mode with our change to ll64
> >  * in the kernel.
> >  */
> > #if defined(__powerpc64__) && !defined(__KERNEL__)
> > # include <asm-generic/int-l64.h>
> > #else
> > # include <asm-generic/int-ll64.h>
> > #endif
> >
> > So ppc64 kernel space uses ll64 and userspace uses l64.
> >
> > This means __u64 is a long for ppc64 but a long long for x86_64. The
> > format errors occurs rightly because a ull is being printed from a ul
> > variable. This will happen to work but its messy.
> 
> Sorry, I'm missing if you are suggesting a solution here... do I need
> to take an alternate approach? The alternative is to patch the
> printfs?

That is the alternative that could probably make it upstream.

I did this mainly to understand what the difference between x86_64 and
ppc64 was so in future I can better spot problem areas and perhaps
others can too.

I'll take the patch but I'd like to include more information about the
reason for the problem in the commit message and strongly hint that it
would be great to get these issues fixed upstream, properly.

Cheers,

Richard
Khem Raj - Oct. 5, 2011, 7:21 p.m.
On Wed, Oct 5, 2011 at 4:34 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2011-10-04 at 21:52 +0000, McClintock Matthew-B29882 wrote:
>> On Mon, Oct 3, 2011 at 4:17 PM, Khem Raj <raj.khem@gmail.com> wrote:
>> >> I can't find a log of this anywhere, it's probably the default
>> >> compiler warnings for our platform are different?
>> >
>> > It would be handy to understand them since I dont see gcc differentiating
>> > formats between ppc64 and x86_64 which both are 64bit hosts. But obviously
>> > there is something missing.
>>
>> I finally got around to rerunning this without my patch and the errors
>> are below.
>
> I dug into this a bit. To quote asm/types.h:
>
> /*
>  * This is here because we used to use l64 for 64bit powerpc
>  * and we don't want to impact user mode with our change to ll64
>  * in the kernel.
>  */
> #if defined(__powerpc64__) && !defined(__KERNEL__)
> # include <asm-generic/int-l64.h>
> #else
> # include <asm-generic/int-ll64.h>
> #endif
>
> So ppc64 kernel space uses ll64 and userspace uses l64.
>

thanks this is what I was interested in.

> This means __u64 is a long for ppc64 but a long long for x86_64. The
> format errors occurs rightly because a ull is being printed from a ul
> variable. This will happen to work but its messy.

havent seen the code but  if its printf like then PRIu64 macro could help.

>
> Cheers,
>
> Richard
>
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
McClintock Matthew-B29882 - Oct. 5, 2011, 9:15 p.m.
On Wed, Oct 5, 2011 at 2:21 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 4:34 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Tue, 2011-10-04 at 21:52 +0000, McClintock Matthew-B29882 wrote:
>>> On Mon, Oct 3, 2011 at 4:17 PM, Khem Raj <raj.khem@gmail.com> wrote:
>>> >> I can't find a log of this anywhere, it's probably the default
>>> >> compiler warnings for our platform are different?
>>> >
>>> > It would be handy to understand them since I dont see gcc differentiating
>>> > formats between ppc64 and x86_64 which both are 64bit hosts. But obviously
>>> > there is something missing.
>>>
>>> I finally got around to rerunning this without my patch and the errors
>>> are below.
>>
>> I dug into this a bit. To quote asm/types.h:
>>
>> /*
>>  * This is here because we used to use l64 for 64bit powerpc
>>  * and we don't want to impact user mode with our change to ll64
>>  * in the kernel.
>>  */
>> #if defined(__powerpc64__) && !defined(__KERNEL__)
>> # include <asm-generic/int-l64.h>
>> #else
>> # include <asm-generic/int-ll64.h>
>> #endif
>>
>> So ppc64 kernel space uses ll64 and userspace uses l64.
>>
>
> thanks this is what I was interested in.
>
>> This means __u64 is a long for ppc64 but a long long for x86_64. The
>> format errors occurs rightly because a ull is being printed from a ul
>> variable. This will happen to work but its messy.
>
> havent seen the code but  if its printf like then PRIu64 macro could help.

Well there is an upstream fix for this as well... I've learned my
lesson now to check upstream repo's..

-M

Patch

diff --git a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
index 5d29ae7..1c83f89 100644
--- a/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
+++ b/meta/recipes-extended/mdadm/mdadm_3.2.2.bb
@@ -8,7 +8,7 @@  LIC_FILES_CHKSUM = "file://COPYING;md5=0636e73ff0215e8d672dc4c32c317bb3 \
                     file://mdmon.c;beginline=4;endline=18;md5=af7d8444d9c4d3e5c7caac0d9d34039d \
                     file://mdadm.h;beglinlne=4;endline=22;md5=462bc9936ac0d3da110191a3f9994161"
 
-PR = "r0"
+PR = "r1"
 
 SRC_URI = "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.bz2"
 
@@ -16,6 +16,7 @@  SRC_URI[md5sum] = "12ee2fbf3beddb60601fb7a4c4905651"
 SRC_URI[sha256sum] = "0d1a04e688b082bc88846e3f524abd50bc782b6ffc06123140f7d358c8f9b906"
 
 CFLAGS += "-fno-strict-aliasing"
+EXTRA_OEMAKE_append_powerpc64 = "CXFLAGS=-Wno-error=format"
 
 inherit autotools