linux-next: ftrace tree build failure

linux-next: ftrace tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

net/wireless/reg.c:348:29: error: macro "if" passed 2 arguments, but takes just 1
net/wireless/reg.c: In function 'ignore_request':
net/wireless/reg.c:349: error: expected '(' before 'return'

This is clearly a bug in in the above file and should be fixed there, so
I have applied the following patch for today.  I expect some patch to fix
this to appear in the wireless-current or net-current tree very soon.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 626dbb6..648c765 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -343,9 +343,7 @@ static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
 				return 0;
 			return -EALREADY;
 		}
-		if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)),
-				"Invalid Country IE regulatory hint passed "
-				"to the wireless core\n")
+		if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)))
 			return -EINVAL;
 		/* We ignore Country IE hints for now, as we haven't yet
 		 * added the dot11MultiDomainCapabilityEnabled flag

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Tue, 2008-11-25 at 14:07 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> net/wireless/reg.c:348:29: error: macro "if" passed 2 arguments, but takes just 1
> net/wireless/reg.c: In function 'ignore_request':
> net/wireless/reg.c:349: error: expected '(' before 'return'
> 
> This is clearly a bug in in the above file and should be fixed there, so
> I have applied the following patch for today.  I expect some patch to fix
> this to appear in the wireless-current or net-current tree very soon.
> 

Ingo already has this in his queue too.

http://marc.info/?l=linux-kernel&m=122745649425646&w=2

-- Steve

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> net/wireless/reg.c:348:29: error: macro "if" passed 2 arguments, but takes just 1
> net/wireless/reg.c: In function 'ignore_request':
> net/wireless/reg.c:349: error: expected '(' before 'return'
> 
> This is clearly a bug in in the above file and should be fixed there, so
> I have applied the following patch for today.  I expect some patch to fix
> this to appear in the wireless-current or net-current tree very soon.
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 626dbb6..648c765 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -343,9 +343,7 @@ static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
>  				return 0;
>  			return -EALREADY;
>  		}
> -		if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)),
> -				"Invalid Country IE regulatory hint passed "
> -				"to the wireless core\n")
> +		if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)))

this fix is wrong - the intention of that line was to add a proper 
WARN(), not a WARN_ON(). Your change loses the message that WARN() 
prints.

i posted a tested fix for this two days ago:

  http://marc.info/?l=linux-kernel&m=122745649425646&w=2

see it also below.

	Ingo

--------------->
>From de734ba59d249ee785a1ead600e5a6ea1dd62639 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Sun, 23 Nov 2008 20:09:54 +0100
Subject: [PATCH] net/wireless/reg.c: fix

fix:

  net/wireless/reg.c:348:29: error: macro "if" passed 2 arguments, but takes just 1

triggered by the branch-tracer.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
 # Please enter the commit message for your changes. Lines starting
---
 net/wireless/reg.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 626dbb6..eb3b1a9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -343,9 +343,9 @@ static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
 				return 0;
 			return -EALREADY;
 		}
-		if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)),
+		if (WARN(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2),
 				"Invalid Country IE regulatory hint passed "
-				"to the wireless core\n")
+				"to the wireless core\n"))
 			return -EINVAL;
 		/* We ignore Country IE hints for now, as we haven't yet
 		 * added the dot11MultiDomainCapabilityEnabled flag

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

Hi Ingo,

On Tue, 25 Nov 2008 08:28:01 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> this fix is wrong - the intention of that line was to add a proper 
> WARN(), not a WARN_ON(). Your change loses the message that WARN() 
> prints.
> 
> i posted a tested fix for this two days ago:
> 
>   http://marc.info/?l=linux-kernel&m=122745649425646&w=2
> 
> see it also below.

Yeah, Steve told me.  However, it might have been useful to tell the
wireless maintainer, the network maintainer and the original author (and
probably netdev - which I also left off).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Ingo,
> 
> On Tue, 25 Nov 2008 08:28:01 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > this fix is wrong - the intention of that line was to add a proper 
> > WARN(), not a WARN_ON(). Your change loses the message that WARN() 
> > prints.
> > 
> > i posted a tested fix for this two days ago:
> > 
> >   http://marc.info/?l=linux-kernel&m=122745649425646&w=2
> > 
> > see it also below.
> 
> Yeah, Steve told me.  However, it might have been useful to tell the 
> wireless maintainer, the network maintainer and the original author 
> (and probably netdev - which I also left off).

yep, i too forgot to add full Cc:s (it was in the context of a tracing 
discussion) - but lkml was Cc:-ed so it wasnt actually lost.

	Ingo

Re: linux-next: ftrace tree build failure

[John W. Linville]

On Tue, Nov 25, 2008 at 09:04:10AM +0100, Ingo Molnar wrote:
> 
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi Ingo,
> > 
> > On Tue, 25 Nov 2008 08:28:01 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> > >
> > > this fix is wrong - the intention of that line was to add a proper 
> > > WARN(), not a WARN_ON(). Your change loses the message that WARN() 
> > > prints.
> > > 
> > > i posted a tested fix for this two days ago:
> > > 
> > >   http://marc.info/?l=linux-kernel&m=122745649425646&w=2
> > > 
> > > see it also below.
> > 
> > Yeah, Steve told me.  However, it might have been useful to tell the 
> > wireless maintainer, the network maintainer and the original author 
> > (and probably netdev - which I also left off).
> 
> yep, i too forgot to add full Cc:s (it was in the context of a tracing 
> discussion) - but lkml was Cc:-ed so it wasnt actually lost.

I have the patch now...thanks!

Now, if I only knew why my build didn't fail with the original code...

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

Re: linux-next: ftrace tree build failure

[Luis R. Rodriguez]

On Tue, Nov 25, 2008 at 06:57:01AM -0800, John W. Linville wrote:
> On Tue, Nov 25, 2008 at 09:04:10AM +0100, Ingo Molnar wrote:
> >
> > * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > > Hi Ingo,
> > >
> > > On Tue, 25 Nov 2008 08:28:01 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> > > >
> > > > this fix is wrong - the intention of that line was to add a proper
> > > > WARN(), not a WARN_ON(). Your change loses the message that WARN()
> > > > prints.
> > > >
> > > > i posted a tested fix for this two days ago:
> > > >
> > > >   http://marc.info/?l=linux-kernel&m=122745649425646&w=2
> > > >
> > > > see it also below.
> > >
> > > Yeah, Steve told me.  However, it might have been useful to tell the
> > > wireless maintainer, the network maintainer and the original author
> > > (and probably netdev - which I also left off).
> >
> > yep, i too forgot to add full Cc:s (it was in the context of a tracing
> > discussion) - but lkml was Cc:-ed so it wasnt actually lost.
> 
> I have the patch now...thanks!
> 
> Now, if I only knew why my build didn't fail with the original code...

Same here I run tests with sparse too prior to sending patches.

  Luis

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Hi,

On Tue, 25 Nov 2008 10:46:30 -0800 "Luis R. Rodriguez" <lrodriguez@atheros.com> wrote:
>
> On Tue, Nov 25, 2008 at 06:57:01AM -0800, John W. Linville wrote:
> > I have the patch now...thanks!
> > 
> > Now, if I only knew why my build didn't fail with the original code...
> 
> Same here I run tests with sparse too prior to sending patches.

It is valid C.

	if (a, b)

you evaluate a and discard it then evaluate b and that is the value of
the conditional expression.  In this case b is just a string which is
effectively a non NULL "const char *" which is just non zero as far as if
is concerned. So the conditional expression evaluates to (constant)
true.  So this was, in fact a bug ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Luis R. Rodriguez]

On Tue, Nov 25, 2008 at 04:08:47PM -0800, Stephen Rothwell wrote:
> Hi,
> 
> On Tue, 25 Nov 2008 10:46:30 -0800 "Luis R. Rodriguez" <lrodriguez@atheros.com> wrote:
> >
> > On Tue, Nov 25, 2008 at 06:57:01AM -0800, John W. Linville wrote:
> > > I have the patch now...thanks!
> > > 
> > > Now, if I only knew why my build didn't fail with the original code...
> > 
> > Same here I run tests with sparse too prior to sending patches.
> 
> It is valid C.
> 
> 	if (a, b)
> 
> you evaluate a and discard it then evaluate b and that is the value of
> the conditional expression.  In this case b is just a string which is
> effectively a non NULL "const char *" which is just non zero as far as if
> is concerned. So the conditional expression evaluates to (constant)
> true.  So this was, in fact a bug ...

Thanks, and good catch :)

  Luis

linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

init/main.c:653: undefined reference to `kmemtrace_init'

I have cherry-picked the fix from the tip/master tree.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> init/main.c:653: undefined reference to `kmemtrace_init'
> 
> I have cherry-picked the fix from the tip/master tree.

thanks! I've pushed out the latest tracing tree so it should be resolved 
in your next iteration.

	Ingo

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

Hi Ingo,

On Wed, 7 Jan 2009 10:55:05 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> thanks! I've pushed out the latest tracing tree so it should be resolved 
> in your next iteration.

Thanks.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

linux-next: ftrace tree build failure

[Stephen Rothwell]

Hi Ingo,

Today's linux-next build (sparc defconfig) failed like this:

mm/slab.c:105:31: error: tracing/kmemtrace.h: No such file or directory

Caused by commit 3fd4bc015ef879a7d2b955ce97fb125e3a51ba7e
("tracing/kmemtrace: export kmemtrace_mark_alloc_node() /
kmemtrace_mark_free()") from the ftrace tree.

For a "build fix", this didn't :-)

I didn't revert the ftrace tree today, but added the patch below.  A bit
more care, please.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 2 Jan 2009 19:25:21 +1100
Subject: [PATCH] tracing/kmemtrace: fix for build fix

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 mm/slab.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 7241a19..ae76de2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -102,7 +102,7 @@
 #include	<linux/cpu.h>
 #include	<linux/sysctl.h>
 #include	<linux/module.h>
-#include	<tracing/kmemtrace.h>
+#include	<trace/kmemtrace.h>
 #include	<linux/rcupdate.h>
 #include	<linux/string.h>
 #include	<linux/uaccess.h>
-- 
1.6.0.5

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Ingo,
> 
> Today's linux-next build (sparc defconfig) failed like this:
> 
> mm/slab.c:105:31: error: tracing/kmemtrace.h: No such file or directory
> 
> Caused by commit 3fd4bc015ef879a7d2b955ce97fb125e3a51ba7e
> ("tracing/kmemtrace: export kmemtrace_mark_alloc_node() /
> kmemtrace_mark_free()") from the ftrace tree.
> 
> For a "build fix", this didn't :-)
> 
> I didn't revert the ftrace tree today, but added the patch below.  A bit
> more care, please.

this has been fixed already a few days ago (if you google for the build 
failure string it will send you straight to the fix patch) - i just didnt 
push it out to the -next branches yet during the hollidays. (did that now)

Generally if you see a problem with our trees you can check whether an 
issue is still present in tip/master:

  http://people.redhat.com/mingo/tip.git/README

before duplicating fixing effort and reporting it as a genuine issue - 
especially in a busy timeframe like the merge window when the _next_ merge 
window is the least of our worries.

Thanks,

	Ingo

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

Hi Ingo,

On Fri, 2 Jan 2009 10:14:56 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> this has been fixed already a few days ago (if you google for the build 
> failure string it will send you straight to the fix patch) - i just didnt 
> push it out to the -next branches yet during the hollidays. (did that now)

Thanks.

> Generally if you see a problem with our trees you can check whether an 
> issue is still present in tip/master:
> 
>   http://people.redhat.com/mingo/tip.git/README

I'll try to remember ...

[I wish you had stopped there]

> before duplicating fixing effort and reporting it as a genuine issue - 

It broke a linux-next build - just how genuine does an issue have to be?
Even if I knew you had already fixed it, I would still have reported it
(although saying where I got the fix from) so that others know why I have
had to apply a patch.

> especially in a busy timeframe like the merge window when the _next_ merge 
> window is the least of our worries.

During the merge window, linux-next is not concerned with the _next_
merge window (as my previous reminder to people spelt out), but very much
with the current merge window.  What you don't seem to realise is that
others use linux-next during the merge window to do integration testing
after fixing up their trees to cope with stuff that Linus has already
merged (in order - as much as possible - to not break Linus' tree).  So
when you break the linux-next tree, you make life harder for them.

So, how much of your time would have been "wasted" remembering to put
that fix into the linux-next branch compared to my time and the time of
others?

And don't bother responding (especially privately) I don't need the
aggravation.  If you feel a need to reply coming on, just delete this
email instead and pretend I didn't write it.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Ingo,
> 
> On Fri, 2 Jan 2009 10:14:56 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > this has been fixed already a few days ago (if you google for the build 
> > failure string it will send you straight to the fix patch) - i just didnt 
> > push it out to the -next branches yet during the hollidays. (did that now)
> 
> Thanks.
> 
> > Generally if you see a problem with our trees you can check whether an 
> > issue is still present in tip/master:
> > 
> >   http://people.redhat.com/mingo/tip.git/README
> 
> I'll try to remember ...
> 
> [I wish you had stopped there]
> 
> > before duplicating fixing effort and reporting it as a genuine issue - 
> 
> It broke a linux-next build - just how genuine does an issue have to be?

as genuine i mean 'new, unknown issue' - not an issue that has been 
reported to lkml already by testers and has been fixed already.

The thing is, i wouldnt have said that had you not said:

> > [...] A bit more care, please.

i can do without such suggestions in the merge window really - it looked 
rather condescending to me. A lkml search or a trivial google for the 
build failure pattern would have given you a working patch.

	Ingo

linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

arch/x86/boot/tty.c: Assembler messages:
arch/x86/boot/tty.c:76: Error: bad register name `%dil'

I have no idea what caused this (maybe a compiler option?) so I have
dropped the ftrace tree for today.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Tue, 2008-11-25 at 14:21 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> arch/x86/boot/tty.c: Assembler messages:
> arch/x86/boot/tty.c:76: Error: bad register name `%dil'
> 
> I have no idea what caused this (maybe a compiler option?) so I have
> dropped the ftrace tree for today.
> 

Stephen,

Could you post a git branch that had this failure, so I can compile it
to see what happened?

Thanks,

-- Steve

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

Hi Steve,

On Mon, 24 Nov 2008 22:48:51 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
>
> Could you post a git branch that had this failure, so I can compile it
> to see what happened?

ssh://master.kernel.org/~sfr/ftrace master

to get the failure, I did:

make ARCH=x86_64 allmodconfig
make ARCH=x86_64 arch/x86/boot/tty.o

My machine is Debian sid.

$ gcc --version
gcc (Debian 4.3.2-1) 4.3.2
$ ld --version
GNU ld (GNU Binutils for Debian) 2.18.0.20080103

The original failure was with a gcc 4.3.2 cross compiled for a powerpc
host and a binutils 2.19.

You can see which trees are merged by looking for commits by me.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Tue, 2008-11-25 at 15:13 +1100, Stephen Rothwell wrote:
> Hi Steve,
> 
> On Mon, 24 Nov 2008 22:48:51 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
> >
> > Could you post a git branch that had this failure, so I can compile it
> > to see what happened?
> 
> ssh://master.kernel.org/~sfr/ftrace master
> 
> to get the failure, I did:
> 
> make ARCH=x86_64 allmodconfig
> make ARCH=x86_64 arch/x86/boot/tty.o
> 
> My machine is Debian sid.
> 
> $ gcc --version
> gcc (Debian 4.3.2-1) 4.3.2
> $ ld --version
> GNU ld (GNU Binutils for Debian) 2.18.0.20080103
> 
> The original failure was with a gcc 4.3.2 cross compiled for a powerpc
> host and a binutils 2.19.
> 
> You can see which trees are merged by looking for commits by me.

This may be a compiler bug, or something introduced by the compiler. I
compiled it fine first with gcc 4.0.2, but then tried with the same
compiler you used, and got the same error as you.

I'll look more at it.

Thanks,

-- Steve

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

Hi Steve,

On Mon, 24 Nov 2008 23:31:33 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
>
> This may be a compiler bug, or something introduced by the compiler. I
> compiled it fine first with gcc 4.0.2, but then tried with the same
> compiler you used, and got the same error as you.

A quick bisect points to this commit:

commit 2bcd521a684cc94befbe2ce7d5b613c841b0d304
Author: Steven Rostedt <srostedt@redhat.com>
Date:   Fri Nov 21 01:30:54 2008 -0500

    trace: profile all if conditionals

I guess something in there is tickling the compiler ...
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Tue, 2008-11-25 at 16:09 +1100, Stephen Rothwell wrote:
> Hi Steve,
> 
> On Mon, 24 Nov 2008 23:31:33 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
> >
> > This may be a compiler bug, or something introduced by the compiler. I
> > compiled it fine first with gcc 4.0.2, but then tried with the same
> > compiler you used, and got the same error as you.
> 
> A quick bisect points to this commit:
> 
> commit 2bcd521a684cc94befbe2ce7d5b613c841b0d304
> Author: Steven Rostedt <srostedt@redhat.com>
> Date:   Fri Nov 21 01:30:54 2008 -0500
> 
>     trace: profile all if conditionals
> 
> I guess something in there is tickling the compiler ...

Yep, it was the wrong constraint used in the asm. Googling around, I
found a similar bug listed in gcc, which was not a bug.

Patch has been sent to fix this.

Ah, I forgot to Cc Peter on that patch.

I'll bounce it to him ;-)

-- Steve

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

Hi Steve,

On Tue, 25 Nov 2008 00:44:16 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
>
> Yep, it was the wrong constraint used in the asm. Googling around, I
> found a similar bug listed in gcc, which was not a bug.

Amazing what we trip over :-(

> Patch has been sent to fix this.

Can you put that patch in your tree as well, or will it reach linux-next
quickly enough anyway?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Steve,
> 
> On Tue, 25 Nov 2008 00:44:16 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
> >
> > Yep, it was the wrong constraint used in the asm. Googling around, I
> > found a similar bug listed in gcc, which was not a bug.
> 
> Amazing what we trip over :-(
> 
> > Patch has been sent to fix this.
> 
> Can you put that patch in your tree as well, or will it reach 
> linux-next quickly enough anyway?

i've applied the assembly constraint fix to the x86 tree and i've 
integrated a new version of auto-x86-next which includes the fix, and 
pushed it out.

Thanks,

	Ingo

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

Hi Ingo,

On Tue, 25 Nov 2008 08:14:36 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> i've applied the assembly constraint fix to the x86 tree and i've 
> integrated a new version of auto-x86-next which includes the fix, and 
> pushed it out.

Thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Tue, 2008-11-25 at 15:13 +1100, Stephen Rothwell wrote:
> Hi Steve,
> 
> On Mon, 24 Nov 2008 22:48:51 -0500 Steven Rostedt <srostedt@redhat.com> wrote:
> >
> > Could you post a git branch that had this failure, so I can compile it
> > to see what happened?
> 
> ssh://master.kernel.org/~sfr/ftrace master
> 
> to get the failure, I did:
> 
> make ARCH=x86_64 allmodconfig
> make ARCH=x86_64 arch/x86/boot/tty.o
> 
> My machine is Debian sid.
> 
> $ gcc --version
> gcc (Debian 4.3.2-1) 4.3.2
> $ ld --version
> GNU ld (GNU Binutils for Debian) 2.18.0.20080103
> 
> The original failure was with a gcc 4.3.2 cross compiled for a powerpc
> host and a binutils 2.19.
> 
> You can see which trees are merged by looking for commits by me.

I traced it down to this.

It compiles without -Os.

But I leave -Os in...

Doing a make arch/x86/boot/tty.i  I get this:

static int kbd_pending(void)
{
 u8 pending;
 asm volatile("int $0x16; setnz %0"
       : "=rm" (pending)
       : "a" (0x0100));
 return pending;
}

void kbd_flush(void)
{
 for (;;) {
  if (__builtin_constant_p((!kbd_pending())) ? !!(!kbd_pending()) :
({ int ______r; static struct ftrace_branch_data
__attribute__((__aligned__(4)))
__attribute__((section("_ftrace_branch"))) ______f = { .func =
__func__, .file = "arch/x86/boot/tty.c", .line = 85, }; ______r = !!(!
kbd_pending()); if (______r) ______f.hit++; else ______f.miss++;
______r; }))
   break;
  getchar();
 }
}


By removing the "static" and "__attribute__((section...))" which is
needed to remove static. It compiles.

It also compiles if I only remove the "if (______r) ______f.hit++; else
______f.miss++;

There's something buggy with gcc where it can not handle -Os and this
combination of having this static variable in the if statement.

-- Steve

linux-next: ftrace tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

kernel/trace/ftrace.c: In function 'ftrace_record_ip':
kernel/trace/ftrace.c:416: error: incompatible type for argument 1 of '_spin_lock_irqsave'
kernel/trace/ftrace.c:433: error: incompatible type for argument 1 of '_spin_lock_irqsave'

(This is after I have disabled CONFIG_FTRACE_MCOUNT_RECORD)

Introduced by commit 6dad8e07f4c10b17b038e84d29f3ca41c2e55cd0 ("ftrace:
add necessary locking for ftrace records").  I have applied the following
patch.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 25 Aug 2008 13:06:46 +1000
Subject: [PATCH] ftrace: fix locking when CONFIG_FTRACE_MCOUNT_RECORD is not set

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/trace/ftrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index dcc8f6b..2bc3eb5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -161,8 +161,8 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
  * not recorded via the compilation.
  */
 static DEFINE_SPINLOCK(ftrace_hash_lock);
-#define ftrace_hash_lock(flags)	  spin_lock_irqsave(ftrace_hash_lock, flags)
-#define ftrace_hash_unlock(flags) spin_lock_irqsave(ftrace_hash_lock, flags)
+#define ftrace_hash_lock(flags)	  spin_lock_irqsave(&ftrace_hash_lock, flags)
+#define ftrace_hash_unlock(flags) spin_lock_irqsave(&ftrace_hash_lock, flags)
 #else
 /* This is protected via the ftrace_lock with MCOUNT_RECORD. */
 #define ftrace_hash_lock(flags)   do { (void)flags; } while (0)
-- 
1.5.6.3

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> kernel/trace/ftrace.c: In function 'ftrace_record_ip':
> kernel/trace/ftrace.c:416: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> kernel/trace/ftrace.c:433: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> 
> (This is after I have disabled CONFIG_FTRACE_MCOUNT_RECORD)

hm, how have you disabled it? It's not an interactive option and it's 
always enabled when CONFIG_DYNAMIC_FTRACE is enabled. (that's how that 
build failure never got discovered)

> Introduced by commit 6dad8e07f4c10b17b038e84d29f3ca41c2e55cd0 ("ftrace:
> add necessary locking for ftrace records").  I have applied the following
> patch.

thanks, applied - the fix makes sense.

	Ingo

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> > kernel/trace/ftrace.c:416: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> > kernel/trace/ftrace.c:433: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> > 
> > (This is after I have disabled CONFIG_FTRACE_MCOUNT_RECORD)
> 
> hm, how have you disabled it? It's not an interactive option and it's 
> always enabled when CONFIG_DYNAMIC_FTRACE is enabled. (that's how that 
> build failure never got discovered)

ah, i see the other thread now, about binutils version dependency. I've 
applied your fix as per the commit below.

	Ingo

>From b2528d2a50942e9d546d3c2e29cde89336053a03 Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 25 Aug 2008 13:08:44 +1000
Subject: [PATCH] ftrace: fix build failure

After disabling FTRACE_MCOUNT_RECORD via a patch, a dormant build
failure surfaced:

 kernel/trace/ftrace.c: In function 'ftrace_record_ip':
 kernel/trace/ftrace.c:416: error: incompatible type for argument 1 of '_spin_lock_irqsave'
 kernel/trace/ftrace.c:433: error: incompatible type for argument 1 of '_spin_lock_irqsave'

Introduced by commit 6dad8e07f4c10b17b038e84d29f3ca41c2e55cd0 ("ftrace:
add necessary locking for ftrace records").

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/trace/ftrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index dcc8f6b..2bc3eb5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -161,8 +161,8 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
  * not recorded via the compilation.
  */
 static DEFINE_SPINLOCK(ftrace_hash_lock);
-#define ftrace_hash_lock(flags)	  spin_lock_irqsave(ftrace_hash_lock, flags)
-#define ftrace_hash_unlock(flags) spin_lock_irqsave(ftrace_hash_lock, flags)
+#define ftrace_hash_lock(flags)	  spin_lock_irqsave(&ftrace_hash_lock, flags)
+#define ftrace_hash_unlock(flags) spin_lock_irqsave(&ftrace_hash_lock, flags)
 #else
 /* This is protected via the ftrace_lock with MCOUNT_RECORD. */
 #define ftrace_hash_lock(flags)   do { (void)flags; } while (0)

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

Hi Ingo,

On Mon, 25 Aug 2008 08:11:01 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > > kernel/trace/ftrace.c:416: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> > > kernel/trace/ftrace.c:433: error: incompatible type for argument 1 of '_spin_lock_irqsave'
> > > 
> > > (This is after I have disabled CONFIG_FTRACE_MCOUNT_RECORD)
> > 
> > hm, how have you disabled it? It's not an interactive option and it's 
> > always enabled when CONFIG_DYNAMIC_FTRACE is enabled. (that's how that 
> > build failure never got discovered)
> 
> ah, i see the other thread now, about binutils version dependency. I've 
> applied your fix as per the commit below.

I knew you would work it out :-)

Looks good, thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

x86_64-unknown-linux-gnu-objcopy: unrecognized option `--globalize-symbol'

Introduced by commit 46483cdbb5dc06d84d9572bac1b90f6744c7fc95 ("ftrace:
create __mcount_loc section").

The currently documented minimum requirements for binutils is 2.12.

$ x86_64-unknown-linux-gnu-objcopy --version
GNU objcopy 2.16.1

For now I have reverted commit c2ffd56692cc4b762c175fad69b70a4ec0dfca3f
("ftrace: enable using mcount recording on x86") since that is the least
intrusive thing to do and the above change depends on
CONFIG_FTRACE_MCOUNT_RECORD.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> x86_64-unknown-linux-gnu-objcopy: unrecognized option `--globalize-symbol'
> 
> Introduced by commit 46483cdbb5dc06d84d9572bac1b90f6744c7fc95 ("ftrace:
> create __mcount_loc section").
> 
> The currently documented minimum requirements for binutils is 2.12.
> 
> $ x86_64-unknown-linux-gnu-objcopy --version
> GNU objcopy 2.16.1
> 
> For now I have reverted commit c2ffd56692cc4b762c175fad69b70a4ec0dfca3f
> ("ftrace: enable using mcount recording on x86") since that is the least
> intrusive thing to do and the above change depends on
> CONFIG_FTRACE_MCOUNT_RECORD.

ok. The binutils i've used for testing is 2.18 and this is indeed a new 
option since 2.17. Steve?

	Ingo

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Mon, 25 Aug 2008, Ingo Molnar wrote:

> 
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi all,
> > 
> > Today's linux-next build (x86_64 allmodconfig) failed like this:
> > 
> > x86_64-unknown-linux-gnu-objcopy: unrecognized option `--globalize-symbol'
> > 
> > Introduced by commit 46483cdbb5dc06d84d9572bac1b90f6744c7fc95 ("ftrace:
> > create __mcount_loc section").
> > 
> > The currently documented minimum requirements for binutils is 2.12.
> > 
> > $ x86_64-unknown-linux-gnu-objcopy --version
> > GNU objcopy 2.16.1
> > 
> > For now I have reverted commit c2ffd56692cc4b762c175fad69b70a4ec0dfca3f
> > ("ftrace: enable using mcount recording on x86") since that is the least
> > intrusive thing to do and the above change depends on
> > CONFIG_FTRACE_MCOUNT_RECORD.
> 
> ok. The binutils i've used for testing is 2.18 and this is indeed a new 
> option since 2.17. Steve?


Ouch! I didn't realize that.

OK, I can test to see if objdump is >= version 2.18 and if it is not, I 
can simply not mark those functions. The current mcount for MCOUNT_RECORD 
is simply a "ret". No saving of registers or anything.

-- Steve

Re: linux-next: ftrace tree build failure

[Steven Rostedt]

On Mon, 25 Aug 2008, Steven Rostedt wrote:
> > 
> > ok. The binutils i've used for testing is 2.18 and this is indeed a new 
> > option since 2.17. Steve?
> 
> 
> Ouch! I didn't realize that.
> 
> OK, I can test to see if objdump is >= version 2.18 and if it is not, I 
> can simply not mark those functions. The current mcount for MCOUNT_RECORD 
> is simply a "ret". No saving of registers or anything.

>= 2.17?

Hmm, I need to find a box with an old version of objcopy.

-- Steve

linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

kernel/trace/ftrace.c: In function 'ftrace_record_ip':
kernel/trace/ftrace.c:345: error: implicit declaration of function 'in_nmi'

Caused by commit 431e94644bde07a327dea17ee9be297c3d44a876 ("ftrace: do
not trace NMI contexts").

in_nmi() is not defined as a function (or macro) anywhere in my kernel
tree, so I assume a patch was missed. More care, please.

I reverted the above commit.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: ftrace tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> kernel/trace/ftrace.c: In function 'ftrace_record_ip':
> kernel/trace/ftrace.c:345: error: implicit declaration of function 'in_nmi'
> 
> Caused by commit 431e94644bde07a327dea17ee9be297c3d44a876 ("ftrace: do
> not trace NMI contexts").
> 
> in_nmi() is not defined as a function (or macro) anywhere in my kernel 
> tree, so I assume a patch was missed. More care, please.
> 
> I reverted the above commit.

hm, it's present in my tree and has been there for a long time:

| commit 0d84b78a606f1562532cd576ee8733caf5a4aed3
| Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
| Date:   Mon May 12 21:21:07 2008 +0200
|
|    x86 NMI-safe INT3 and Page Fault

what happened is that i restructured the integration branches yesterday 
and moved the tracing/nmisafe from auto-ftrace-next to auto-latest - 
without realizing the dependency. I fixed that now and pushed out the 
updated auto-ftrace-next branch.

	Ingo

Re: linux-next: ftrace tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Hi Ingo,

On Fri, 15 Aug 2008 09:56:14 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>
> hm, it's present in my tree and has been there for a long time:
> 
> | commit 0d84b78a606f1562532cd576ee8733caf5a4aed3
> | Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> | Date:   Mon May 12 21:21:07 2008 +0200
> |
> |    x86 NMI-safe INT3 and Page Fault
> 
> what happened is that i restructured the integration branches yesterday 
> and moved the tracing/nmisafe from auto-ftrace-next to auto-latest - 
> without realizing the dependency. I fixed that now and pushed out the 
> updated auto-ftrace-next branch.

OK, thanks.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]