linux-next: tree build failure

linux-next: tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (powerpc ppc44x_defconfig) failed like this:

In file included from arch/powerpc/kvm/booke.c:31:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
In file included from arch/powerpc/kvm/booke.h:26,
                 from arch/powerpc/kvm/booke_emulate.c:23:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant

Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

I applied the following patch for today.  This inline function is
only called from one place in one file ...

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 24 Sep 2009 15:13:20 +1000
Subject: [PATCH] powerpc/kvm: build fix for new BUILD_BUG_ON

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/kvm/timing.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.h b/arch/powerpc/kvm/timing.h
index bb13b1f..4c34099 100644
--- a/arch/powerpc/kvm/timing.h
+++ b/arch/powerpc/kvm/timing.h
@@ -48,7 +48,7 @@ static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type) {}
 static inline void kvmppc_account_exit_stat(struct kvm_vcpu *vcpu, int type)
 {
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	//BUILD_BUG_ON(__builtin_constant_p(type));
 	switch (type) {
 	case EXT_INTR_EXITS:
 		vcpu->stat.ext_intr_exits++;
-- 
1.6.4.3


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

linux-next: tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (powerpc ppc44x_defconfig) failed like this:

In file included from arch/powerpc/kvm/booke.c:31:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
In file included from arch/powerpc/kvm/booke.h:26,
                 from arch/powerpc/kvm/booke_emulate.c:23:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant

Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

I applied the following patch for today.  This inline function is
only called from one place in one file ...

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 24 Sep 2009 15:13:20 +1000
Subject: [PATCH] powerpc/kvm: build fix for new BUILD_BUG_ON

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/kvm/timing.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.h b/arch/powerpc/kvm/timing.h
index bb13b1f..4c34099 100644
--- a/arch/powerpc/kvm/timing.h
+++ b/arch/powerpc/kvm/timing.h
@@ -48,7 +48,7 @@ static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type) {}
 static inline void kvmppc_account_exit_stat(struct kvm_vcpu *vcpu, int type)
 {
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	//BUILD_BUG_ON(__builtin_constant_p(type));
 	switch (type) {
 	case EXT_INTR_EXITS:
 		vcpu->stat.ext_intr_exits++;
-- 
1.6.4.3


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

linux-next: tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (powerpc ppc44x_defconfig) failed like this:

In file included from arch/powerpc/kvm/booke.c:31:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
In file included from arch/powerpc/kvm/booke.h:26,
                 from arch/powerpc/kvm/booke_emulate.c:23:
arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant

Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

I applied the following patch for today.  This inline function is
only called from one place in one file ...

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 24 Sep 2009 15:13:20 +1000
Subject: [PATCH] powerpc/kvm: build fix for new BUILD_BUG_ON

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/kvm/timing.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.h b/arch/powerpc/kvm/timing.h
index bb13b1f..4c34099 100644
--- a/arch/powerpc/kvm/timing.h
+++ b/arch/powerpc/kvm/timing.h
@@ -48,7 +48,7 @@ static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type) {}
 static inline void kvmppc_account_exit_stat(struct kvm_vcpu *vcpu, int type)
 {
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	//BUILD_BUG_ON(__builtin_constant_p(type));
 	switch (type) {
 	case EXT_INTR_EXITS:
 		vcpu->stat.ext_intr_exits++;
-- 
1.6.4.3


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-09-24 at 15:21 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> 
> In file included from arch/powerpc/kvm/booke.c:31:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> In file included from arch/powerpc/kvm/booke.h:26,
>                  from arch/powerpc/kvm/booke_emulate.c:23:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> 
> Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
> ("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

First, I think there is a real bug here, and the code should read like
this (to match the comment):
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	BUILD_BUG_ON(!__builtin_constant_p(type));

However, I get the same build error *both* ways, i.e.
__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
the new BUILD_BUG_ON() macro isn't working...

> I applied the following patch for today.  This inline function is
> only called from one place in one file ...

It's also called via kvmppc_account_exit() from a number of places.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-09-24 at 15:21 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> 
> In file included from arch/powerpc/kvm/booke.c:31:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> In file included from arch/powerpc/kvm/booke.h:26,
>                  from arch/powerpc/kvm/booke_emulate.c:23:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> 
> Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
> ("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

First, I think there is a real bug here, and the code should read like
this (to match the comment):
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	BUILD_BUG_ON(!__builtin_constant_p(type));

However, I get the same build error *both* ways, i.e.
__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
the new BUILD_BUG_ON() macro isn't working...

> I applied the following patch for today.  This inline function is
> only called from one place in one file ...

It's also called via kvmppc_account_exit() from a number of places.

-- 
Hollis Blanchard
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-09-24 at 15:21 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> 
> In file included from arch/powerpc/kvm/booke.c:31:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> In file included from arch/powerpc/kvm/booke.h:26,
>                  from arch/powerpc/kvm/booke_emulate.c:23:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> 
> Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
> ("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

First, I think there is a real bug here, and the code should read like
this (to match the comment):
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	BUILD_BUG_ON(!__builtin_constant_p(type));

However, I get the same build error *both* ways, i.e.
__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
the new BUILD_BUG_ON() macro isn't working...

> I applied the following patch for today.  This inline function is
> only called from one place in one file ...

It's also called via kvmppc_account_exit() from a number of places.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-09-24 at 15:21 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> 
> In file included from arch/powerpc/kvm/booke.c:31:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> In file included from arch/powerpc/kvm/booke.h:26,
>                  from arch/powerpc/kvm/booke_emulate.c:23:
> arch/powerpc/kvm/timing.h: In function 'kvmppc_account_exit_stat':
> arch/powerpc/kvm/timing.h:51: error: bit-field '<anonymous>' width not an integer constant
> 
> Presumably caused by commit 8c87df457cb58fe75b9b893007917cf8095660a0
> ("BUILD_BUG_ON(): fix it and a couple of bogus uses of it").

First, I think there is a real bug here, and the code should read like
this (to match the comment):
 	/* type has to be known at build time for optimization */
-	BUILD_BUG_ON(__builtin_constant_p(type));
+	BUILD_BUG_ON(!__builtin_constant_p(type));

However, I get the same build error *both* ways, i.e.
__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
the new BUILD_BUG_ON() macro isn't working...

> I applied the following patch for today.  This inline function is
> only called from one place in one file ...

It's also called via kvmppc_account_exit() from a number of places.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>First, I think there is a real bug here, and the code should read like
>this (to match the comment):
>    /* type has to be known at build time for optimization */
>-    BUILD_BUG_ON(__builtin_constant_p(type));
>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>
>However, I get the same build error *both* ways, i.e.
>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>the new BUILD_BUG_ON() macro isn't working...

No, at this point of the compilation process it's neither zero nor one,
it's simply considered non-constant by the compiler at that stage
(this builtin is used for optimization, not during parsing, and the
error gets generated when the body of the function gets parsed,
not when code gets generated from it).

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>First, I think there is a real bug here, and the code should read like
>this (to match the comment):
>    /* type has to be known at build time for optimization */
>-    BUILD_BUG_ON(__builtin_constant_p(type));
>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>
>However, I get the same build error *both* ways, i.e.
>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>the new BUILD_BUG_ON() macro isn't working...

No, at this point of the compilation process it's neither zero nor one,
it's simply considered non-constant by the compiler at that stage
(this builtin is used for optimization, not during parsing, and the
error gets generated when the body of the function gets parsed,
not when code gets generated from it).

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>First, I think there is a real bug here, and the code should read like
>this (to match the comment):
>    /* type has to be known at build time for optimization */
>-    BUILD_BUG_ON(__builtin_constant_p(type));
>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>
>However, I get the same build error *both* ways, i.e.
>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>the new BUILD_BUG_ON() macro isn't working...

No, at this point of the compilation process it's neither zero nor one,
it's simply considered non-constant by the compiler at that stage
(this builtin is used for optimization, not during parsing, and the
error gets generated when the body of the function gets parsed,
not when code gets generated from it).

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>First, I think there is a real bug here, and the code should read like
>this (to match the comment):
>    /* type has to be known at build time for optimization */
>-    BUILD_BUG_ON(__builtin_constant_p(type));
>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>
>However, I get the same build error *both* ways, i.e.
>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>the new BUILD_BUG_ON() macro isn't working...

No, at this point of the compilation process it's neither zero nor one,
it's simply considered non-constant by the compiler at that stage
(this builtin is used for optimization, not during parsing, and the
error gets generated when the body of the function gets parsed,
not when code gets generated from it).

Jan

Re: linux-next: tree build failure

[roel kluin]

On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> wrote:
>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>First, I think there is a real bug here, and the code should read like
>>this (to match the comment):
>>    /* type has to be known at build time for optimization */
>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>
>>However, I get the same build error *both* ways, i.e.
>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
>
> Jan

then maybe

if(__builtin_constant_p(type))
        BUILD_BUG_ON(1);

would work?

Roel

Re: linux-next: tree build failure

[roel kluin]

On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>First, I think there is a real bug here, and the code should read like
>>this (to match the comment):
>>    /* type has to be known at build time for optimization */
>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>
>>However, I get the same build error *both* ways, i.e.
>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
>
> Jan

then maybe

if(__builtin_constant_p(type))
        BUILD_BUG_ON(1);

would work?

Roel
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[roel kluin]

On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> wrote:
>>>> Hollis Blanchard =A009/29/09 2:00 AM >>>
>>First, I think there is a real bug here, and the code should read like
>>this (to match the comment):
>> =A0 =A0/* type has to be known at build time for optimization */
>>- =A0 =A0BUILD_BUG_ON(__builtin_constant_p(type));
>>+ =A0 =A0BUILD_BUG_ON(!__builtin_constant_p(type));
>>
>>However, I get the same build error *both* ways, i.e.
>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
>
> Jan

then maybe

if(__builtin_constant_p(type))
        BUILD_BUG_ON(1);

would work?

Roel

Re: linux-next: tree build failure

[roel kluin]

On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> wrote:
>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>First, I think there is a real bug here, and the code should read like
>>this (to match the comment):
>>    /* type has to be known at build time for optimization */
>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>
>>However, I get the same build error *both* ways, i.e.
>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
>
> Jan

then maybe

if(__builtin_constant_p(type))
        BUILD_BUG_ON(1);

would work?

Roel

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >First, I think there is a real bug here, and the code should read like
> >this (to match the comment):
> >    /* type has to be known at build time for optimization */
> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >
> >However, I get the same build error *both* ways, i.e.
> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >the new BUILD_BUG_ON() macro isn't working...
> 
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).

I think I see what you're saying. Do you have a fix to suggest?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >First, I think there is a real bug here, and the code should read like
> >this (to match the comment):
> >    /* type has to be known at build time for optimization */
> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >
> >However, I get the same build error *both* ways, i.e.
> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >the new BUILD_BUG_ON() macro isn't working...
> 
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).

I think I see what you're saying. Do you have a fix to suggest?

-- 
Hollis Blanchard
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >First, I think there is a real bug here, and the code should read like
> >this (to match the comment):
> >    /* type has to be known at build time for optimization */
> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >
> >However, I get the same build error *both* ways, i.e.
> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >the new BUILD_BUG_ON() macro isn't working...
> 
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).

I think I see what you're saying. Do you have a fix to suggest?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >First, I think there is a real bug here, and the code should read like
> >this (to match the comment):
> >    /* type has to be known at build time for optimization */
> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >
> >However, I get the same build error *both* ways, i.e.
> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >the new BUILD_BUG_ON() macro isn't working...
> 
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).

I think I see what you're saying. Do you have a fix to suggest?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Jan Beulich]

>>> roel kluin <roel.kluin@gmail.com> 29.09.09 11:51 >>>
>On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> wrote:
>>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>>First, I think there is a real bug here, and the code should read like
>>>this (to match the comment):
>>>    /* type has to be known at build time for optimization */
>>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>>
>>>However, I get the same build error *both* ways, i.e.
>>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>>the new BUILD_BUG_ON() macro isn't working...
>>
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>>
>> Jan
>
>then maybe
>
>if(__builtin_constant_p(type))
>        BUILD_BUG_ON(1);
>
>would work?

Definitely not - this would result in the compiler *always* generating an
error.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> roel kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 29.09.09 11:51 >>>
>On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org> wrote:
>>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>>First, I think there is a real bug here, and the code should read like
>>>this (to match the comment):
>>>    /* type has to be known at build time for optimization */
>>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>>
>>>However, I get the same build error *both* ways, i.e.
>>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>>the new BUILD_BUG_ON() macro isn't working...
>>
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>>
>> Jan
>
>then maybe
>
>if(__builtin_constant_p(type))
>        BUILD_BUG_ON(1);
>
>would work?

Definitely not - this would result in the compiler *always* generating an
error.

Jan

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[Jan Beulich]

>>> roel kluin <roel.kluin@gmail.com> 29.09.09 11:51 >>>
>On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> =
wrote:
>>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>>First, I think there is a real bug here, and the code should read like
>>>this (to match the comment):
>>>    /* type has to be known at build time for optimization */
>>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>>
>>>However, I get the same build error *both* ways, i.e.
>>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>>the new BUILD_BUG_ON() macro isn't working...
>>
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>>
>> Jan
>
>then maybe
>
>if(__builtin_constant_p(type))
>        BUILD_BUG_ON(1);
>
>would work?

Definitely not - this would result in the compiler *always* generating an
error.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> roel kluin <roel.kluin@gmail.com> 29.09.09 11:51 >>>
>On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> wrote:
>>>>> Hollis Blanchard  09/29/09 2:00 AM >>>
>>>First, I think there is a real bug here, and the code should read like
>>>this (to match the comment):
>>>    /* type has to be known at build time for optimization */
>>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>>
>>>However, I get the same build error *both* ways, i.e.
>>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>>the new BUILD_BUG_ON() macro isn't working...
>>
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>>
>> Jan
>
>then maybe
>
>if(__builtin_constant_p(type))
>        BUILD_BUG_ON(1);
>
>would work?

Definitely not - this would result in the compiler *always* generating an
error.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
>On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
>> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
>> >First, I think there is a real bug here, and the code should read like
>> >this (to match the comment):
>> >    /* type has to be known at build time for optimization */
>> >-    BUILD_BUG_ON(__builtin_constant_p(type));
>> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
>> >
>> >However, I get the same build error *both* ways, i.e.
>> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>> >the new BUILD_BUG_ON() macro isn't working...
>> 
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>
>I think I see what you're saying. Do you have a fix to suggest?

The one Rusty suggested the other day may help here. I don't like it
as a drop-in replacement for BUILD_BUG_ON() though (due to it
deferring the error generated to the linking stage), I'd rather view
this as an improvement to MAYBE_BUILD_BUG_ON() (which should
then be used here).

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> 30.09.09 01:39 >>>
>On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
>> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
>> >First, I think there is a real bug here, and the code should read like
>> >this (to match the comment):
>> >    /* type has to be known at build time for optimization */
>> >-    BUILD_BUG_ON(__builtin_constant_p(type));
>> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
>> >
>> >However, I get the same build error *both* ways, i.e.
>> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>> >the new BUILD_BUG_ON() macro isn't working...
>> 
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>
>I think I see what you're saying. Do you have a fix to suggest?

The one Rusty suggested the other day may help here. I don't like it
as a drop-in replacement for BUILD_BUG_ON() though (due to it
deferring the error generated to the linking stage), I'd rather view
this as an improvement to MAYBE_BUILD_BUG_ON() (which should
then be used here).

Jan

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
>On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
>> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
>> >First, I think there is a real bug here, and the code should read like
>> >this (to match the comment):
>> >    /* type has to be known at build time for optimization */
>> >-    BUILD_BUG_ON(__builtin_constant_p(type));
>> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
>> >
>> >However, I get the same build error *both* ways, i.e.
>> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>> >the new BUILD_BUG_ON() macro isn't working...
>>=20
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>
>I think I see what you're saying. Do you have a fix to suggest?

The one Rusty suggested the other day may help here. I don't like it
as a drop-in replacement for BUILD_BUG_ON() though (due to it
deferring the error generated to the linking stage), I'd rather view
this as an improvement to MAYBE_BUILD_BUG_ON() (which should
then be used here).

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
>On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
>> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
>> >First, I think there is a real bug here, and the code should read like
>> >this (to match the comment):
>> >    /* type has to be known at build time for optimization */
>> >-    BUILD_BUG_ON(__builtin_constant_p(type));
>> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
>> >
>> >However, I get the same build error *both* ways, i.e.
>> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>> >the new BUILD_BUG_ON() macro isn't working...
>> 
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>
>I think I see what you're saying. Do you have a fix to suggest?

The one Rusty suggested the other day may help here. I don't like it
as a drop-in replacement for BUILD_BUG_ON() though (due to it
deferring the error generated to the linking stage), I'd rather view
this as an improvement to MAYBE_BUILD_BUG_ON() (which should
then be used here).

Jan

Re: linux-next: tree build failure

[Hollis Blanchard]

On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
> >On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >> >First, I think there is a real bug here, and the code should read like
> >> >this (to match the comment):
> >> >    /* type has to be known at build time for optimization */
> >> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >> >
> >> >However, I get the same build error *both* ways, i.e.
> >> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >> >the new BUILD_BUG_ON() macro isn't working...
> >> 
> >> No, at this point of the compilation process it's neither zero nor one,
> >> it's simply considered non-constant by the compiler at that stage
> >> (this builtin is used for optimization, not during parsing, and the
> >> error gets generated when the body of the function gets parsed,
> >> not when code gets generated from it).
> >
> >I think I see what you're saying. Do you have a fix to suggest?
> 
> The one Rusty suggested the other day may help here. I don't like it
> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> deferring the error generated to the linking stage), I'd rather view
> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> then be used here).

Can you be more specific?

I have no idea what Rusty suggested where. I can't even guess what
MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

All I know is that this used to build...

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> 30.09.09 01:39 >>>
> >On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >> >First, I think there is a real bug here, and the code should read like
> >> >this (to match the comment):
> >> >    /* type has to be known at build time for optimization */
> >> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >> >
> >> >However, I get the same build error *both* ways, i.e.
> >> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >> >the new BUILD_BUG_ON() macro isn't working...
> >> 
> >> No, at this point of the compilation process it's neither zero nor one,
> >> it's simply considered non-constant by the compiler at that stage
> >> (this builtin is used for optimization, not during parsing, and the
> >> error gets generated when the body of the function gets parsed,
> >> not when code gets generated from it).
> >
> >I think I see what you're saying. Do you have a fix to suggest?
> 
> The one Rusty suggested the other day may help here. I don't like it
> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> deferring the error generated to the linking stage), I'd rather view
> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> then be used here).

Can you be more specific?

I have no idea what Rusty suggested where. I can't even guess what
MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

All I know is that this used to build...

-- 
Hollis Blanchard
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: tree build failure

[Hollis Blanchard]

On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
> >On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >> >First, I think there is a real bug here, and the code should read like
> >> >this (to match the comment):
> >> >    /* type has to be known at build time for optimization */
> >> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >> >
> >> >However, I get the same build error *both* ways, i.e.
> >> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >> >the new BUILD_BUG_ON() macro isn't working...
> >> 
> >> No, at this point of the compilation process it's neither zero nor one,
> >> it's simply considered non-constant by the compiler at that stage
> >> (this builtin is used for optimization, not during parsing, and the
> >> error gets generated when the body of the function gets parsed,
> >> not when code gets generated from it).
> >
> >I think I see what you're saying. Do you have a fix to suggest?
> 
> The one Rusty suggested the other day may help here. I don't like it
> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> deferring the error generated to the linking stage), I'd rather view
> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> then be used here).

Can you be more specific?

I have no idea what Rusty suggested where. I can't even guess what
MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

All I know is that this used to build...

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
> >On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >> >>> Hollis Blanchard  09/29/09 2:00 AM >>>
> >> >First, I think there is a real bug here, and the code should read like
> >> >this (to match the comment):
> >> >    /* type has to be known at build time for optimization */
> >> >-    BUILD_BUG_ON(__builtin_constant_p(type));
> >> >+    BUILD_BUG_ON(!__builtin_constant_p(type));
> >> >
> >> >However, I get the same build error *both* ways, i.e.
> >> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >> >the new BUILD_BUG_ON() macro isn't working...
> >> 
> >> No, at this point of the compilation process it's neither zero nor one,
> >> it's simply considered non-constant by the compiler at that stage
> >> (this builtin is used for optimization, not during parsing, and the
> >> error gets generated when the body of the function gets parsed,
> >> not when code gets generated from it).
> >
> >I think I see what you're saying. Do you have a fix to suggest?
> 
> The one Rusty suggested the other day may help here. I don't like it
> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> deferring the error generated to the linking stage), I'd rather view
> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> then be used here).

Can you be more specific?

I have no idea what Rusty suggested where. I can't even guess what
MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

All I know is that this used to build...

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Jan Beulich]

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

>>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
>On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
>> The one Rusty suggested the other day may help here. I don't like it
>> as a drop-in replacement for BUILD_BUG_ON() though (due to it
>> deferring the error generated to the linking stage), I'd rather view
>> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
>> then be used here).
>
>Can you be more specific?
>
>I have no idea what Rusty suggested where. I can't even guess what

I'm attaching Rusty's response I was referring to.

>MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

Agreed - but presumably better than just deleting the bogus instances
altogether...

Jan

[-- Attachment #2: Type: message/rfc822, Size: 2578 bytes --]

From: Rusty Russell <rusty@rustcorp.com.au>
To: "Jan Beulich" <JBeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it
Date: Wed, 23 Sep 2009 10:27:00 +0930
Message-ID: <200909231027.01006.rusty@rustcorp.com.au>

On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> gcc permitting variable length arrays makes the current construct
> used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> if the controlling expression isn't really constant. Instead, this
> patch makes it so that a bit field gets used here. Consequently, those
> uses where the condition isn't really constant now also need fixing.
> 
> Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> if the expression is compile time constant (__builtin_constant_p()
> yields true), the array is still deemed of variable length by gcc, and
> hence the whole expression doesn't have the intended effect.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

We used to use an undefined symbol here; diagnostics are worse but it catches
more stuff.

Perhaps a hybrid is the way to go?

#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
/* If it's a constant, catch it at compile time, otherwise at link time. */
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
	do { 								\
		((void)sizeof(char[1 - 2*!!(condition)]));		\
		if (condition) __build_bug_on_failed = 1;		\
	} while(0)
#endif

Thanks,
Rusty.

Re: linux-next: tree build failure

[Jan Beulich]

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

>>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
>On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
>> The one Rusty suggested the other day may help here. I don't like it
>> as a drop-in replacement for BUILD_BUG_ON() though (due to it
>> deferring the error generated to the linking stage), I'd rather view
>> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
>> then be used here).
>
>Can you be more specific?
>
>I have no idea what Rusty suggested where. I can't even guess what

I'm attaching Rusty's response I was referring to.

>MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

Agreed - but presumably better than just deleting the bogus instances
altogether...

Jan

[-- Attachment #2: Type: message/rfc822, Size: 2578 bytes --]

From: Rusty Russell <rusty@rustcorp.com.au>
To: "Jan Beulich" <JBeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it
Date: Wed, 23 Sep 2009 10:27:00 +0930
Message-ID: <200909231027.01006.rusty@rustcorp.com.au>

On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> gcc permitting variable length arrays makes the current construct
> used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> if the controlling expression isn't really constant. Instead, this
> patch makes it so that a bit field gets used here. Consequently, those
> uses where the condition isn't really constant now also need fixing.
> 
> Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> if the expression is compile time constant (__builtin_constant_p()
> yields true), the array is still deemed of variable length by gcc, and
> hence the whole expression doesn't have the intended effect.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

We used to use an undefined symbol here; diagnostics are worse but it catches
more stuff.

Perhaps a hybrid is the way to go?

#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
/* If it's a constant, catch it at compile time, otherwise at link time. */
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
	do { 								\
		((void)sizeof(char[1 - 2*!!(condition)]));		\
		if (condition) __build_bug_on_failed = 1;		\
	} while(0)
#endif

Thanks,
Rusty.

[-- Attachment #3: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: linux-next: tree build failure

[Jan Beulich]

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

>>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
>On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
>> The one Rusty suggested the other day may help here. I don't like it
>> as a drop-in replacement for BUILD_BUG_ON() though (due to it
>> deferring the error generated to the linking stage), I'd rather view
>> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
>> then be used here).
>
>Can you be more specific?
>
>I have no idea what Rusty suggested where. I can't even guess what

I'm attaching Rusty's response I was referring to.

>MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

Agreed - but presumably better than just deleting the bogus instances
altogether...

Jan

[-- Attachment #2: Type: message/rfc822, Size: 2578 bytes --]

From: Rusty Russell <rusty@rustcorp.com.au>
To: "Jan Beulich" <JBeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it
Date: Wed, 23 Sep 2009 10:27:00 +0930
Message-ID: <200909231027.01006.rusty@rustcorp.com.au>

On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> gcc permitting variable length arrays makes the current construct
> used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> if the controlling expression isn't really constant. Instead, this
> patch makes it so that a bit field gets used here. Consequently, those
> uses where the condition isn't really constant now also need fixing.
> 
> Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> if the expression is compile time constant (__builtin_constant_p()
> yields true), the array is still deemed of variable length by gcc, and
> hence the whole expression doesn't have the intended effect.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

We used to use an undefined symbol here; diagnostics are worse but it catches
more stuff.

Perhaps a hybrid is the way to go?

#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
/* If it's a constant, catch it at compile time, otherwise at link time. */
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
	do { 								\
		((void)sizeof(char[1 - 2*!!(condition)]));		\
		if (condition) __build_bug_on_failed = 1;		\
	} while(0)
#endif

Thanks,
Rusty.

Re: linux-next: tree build failure

[Jan Beulich]

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

>>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
>On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
>> The one Rusty suggested the other day may help here. I don't like it
>> as a drop-in replacement for BUILD_BUG_ON() though (due to it
>> deferring the error generated to the linking stage), I'd rather view
>> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
>> then be used here).
>
>Can you be more specific?
>
>I have no idea what Rusty suggested where. I can't even guess what

I'm attaching Rusty's response I was referring to.

>MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).

Agreed - but presumably better than just deleting the bogus instances
altogether...

Jan

[-- Attachment #2: Type: message/rfc822, Size: 2578 bytes --]

From: Rusty Russell <rusty@rustcorp.com.au>
To: "Jan Beulich" <JBeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it
Date: Wed, 23 Sep 2009 10:27:00 +0930
Message-ID: <200909231027.01006.rusty@rustcorp.com.au>

On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> gcc permitting variable length arrays makes the current construct
> used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> if the controlling expression isn't really constant. Instead, this
> patch makes it so that a bit field gets used here. Consequently, those
> uses where the condition isn't really constant now also need fixing.
> 
> Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> if the expression is compile time constant (__builtin_constant_p()
> yields true), the array is still deemed of variable length by gcc, and
> hence the whole expression doesn't have the intended effect.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

We used to use an undefined symbol here; diagnostics are worse but it catches
more stuff.

Perhaps a hybrid is the way to go?

#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
/* If it's a constant, catch it at compile time, otherwise at link time. */
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
	do { 								\
		((void)sizeof(char[1 - 2*!!(condition)]));		\
		if (condition) __build_bug_on_failed = 1;		\
	} while(0)
#endif

Thanks,
Rusty.

Re: linux-next: tree build failure

[Hollis Blanchard]

Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
also exposes the bug in kvmppc_account_exit_stat(). So to recap:

original: built but didn't work
Jan's: doesn't build
Rusty's: builds and works

Where do you want to go from here?

-- 
Hollis Blanchard
IBM Linux Technology Center

On Mon, 2009-10-05 at 07:58 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
> >On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >> The one Rusty suggested the other day may help here. I don't like it
> >> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> >> deferring the error generated to the linking stage), I'd rather view
> >> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> >> then be used here).
> >
> >Can you be more specific?
> >
> >I have no idea what Rusty suggested where. I can't even guess what
> 
> I'm attaching Rusty's response I was referring to.
> 
> >MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).
> 
> Agreed - but presumably better than just deleting the bogus instances
> altogether...
> 
> Jan
> email message attachment
> > -------- Forwarded Message --------
> > From: Rusty Russell <rusty@rustcorp.com.au>
> > To: Jan Beulich <JBeulich@novell.com>
> > Cc: linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses
> > of it
> > Date: Wed, 23 Sep 2009 10:27:00 +0930
> > 
> > On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> > > gcc permitting variable length arrays makes the current construct
> > > used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> > > if the controlling expression isn't really constant. Instead, this
> > > patch makes it so that a bit field gets used here. Consequently, those
> > > uses where the condition isn't really constant now also need fixing.
> > > 
> > > Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> > > MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> > > if the expression is compile time constant (__builtin_constant_p()
> > > yields true), the array is still deemed of variable length by gcc, and
> > > hence the whole expression doesn't have the intended effect.
> > > 
> > > Signed-off-by: Jan Beulich <jbeulich@novell.com>
> > 
> > We used to use an undefined symbol here; diagnostics are worse but it catches
> > more stuff.
> > 
> > Perhaps a hybrid is the way to go?
> > 
> > #ifndef __OPTIMIZE__
> > #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > #else
> > /* If it's a constant, catch it at compile time, otherwise at link time. */
> > extern int __build_bug_on_failed;
> > #define BUILD_BUG_ON(condition) \
> > 	do { 								\
> > 		((void)sizeof(char[1 - 2*!!(condition)]));		\
> > 		if (condition) __build_bug_on_failed = 1;		\
> > 	} while(0)
> > #endif
> > 
> > Thanks,
> > Rusty.

Re: linux-next: tree build failure

[Hollis Blanchard]

Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
also exposes the bug in kvmppc_account_exit_stat(). So to recap:

original: built but didn't work
Jan's: doesn't build
Rusty's: builds and works

Where do you want to go from here?

-- 
Hollis Blanchard
IBM Linux Technology Center

On Mon, 2009-10-05 at 07:58 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
> >On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >> The one Rusty suggested the other day may help here. I don't like it
> >> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> >> deferring the error generated to the linking stage), I'd rather view
> >> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> >> then be used here).
> >
> >Can you be more specific?
> >
> >I have no idea what Rusty suggested where. I can't even guess what
> 
> I'm attaching Rusty's response I was referring to.
> 
> >MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).
> 
> Agreed - but presumably better than just deleting the bogus instances
> altogether...
> 
> Jan
> email message attachment
> > -------- Forwarded Message --------
> > From: Rusty Russell <rusty@rustcorp.com.au>
> > To: Jan Beulich <JBeulich@novell.com>
> > Cc: linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses
> > of it
> > Date: Wed, 23 Sep 2009 10:27:00 +0930
> > 
> > On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> > > gcc permitting variable length arrays makes the current construct
> > > used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> > > if the controlling expression isn't really constant. Instead, this
> > > patch makes it so that a bit field gets used here. Consequently, those
> > > uses where the condition isn't really constant now also need fixing.
> > > 
> > > Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> > > MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> > > if the expression is compile time constant (__builtin_constant_p()
> > > yields true), the array is still deemed of variable length by gcc, and
> > > hence the whole expression doesn't have the intended effect.
> > > 
> > > Signed-off-by: Jan Beulich <jbeulich@novell.com>
> > 
> > We used to use an undefined symbol here; diagnostics are worse but it catches
> > more stuff.
> > 
> > Perhaps a hybrid is the way to go?
> > 
> > #ifndef __OPTIMIZE__
> > #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > #else
> > /* If it's a constant, catch it at compile time, otherwise at link time. */
> > extern int __build_bug_on_failed;
> > #define BUILD_BUG_ON(condition) \
> > 	do { 								\
> > 		((void)sizeof(char[1 - 2*!!(condition)]));		\
> > 		if (condition) __build_bug_on_failed = 1;		\
> > 	} while(0)
> > #endif
> > 
> > Thanks,
> > Rusty.

Re: linux-next: tree build failure

[Hollis Blanchard]

Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
also exposes the bug in kvmppc_account_exit_stat(). So to recap:

original: built but didn't work
Jan's: doesn't build
Rusty's: builds and works

Where do you want to go from here?

-- 
Hollis Blanchard
IBM Linux Technology Center

On Mon, 2009-10-05 at 07:58 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
> >On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
> >> The one Rusty suggested the other day may help here. I don't like it
> >> as a drop-in replacement for BUILD_BUG_ON() though (due to it
> >> deferring the error generated to the linking stage), I'd rather view
> >> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
> >> then be used here).
> >
> >Can you be more specific?
> >
> >I have no idea what Rusty suggested where. I can't even guess what
> 
> I'm attaching Rusty's response I was referring to.
> 
> >MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).
> 
> Agreed - but presumably better than just deleting the bogus instances
> altogether...
> 
> Jan
> email message attachment
> > -------- Forwarded Message --------
> > From: Rusty Russell <rusty@rustcorp.com.au>
> > To: Jan Beulich <JBeulich@novell.com>
> > Cc: linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses
> > of it
> > Date: Wed, 23 Sep 2009 10:27:00 +0930
> > 
> > On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> > > gcc permitting variable length arrays makes the current construct
> > > used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> > > if the controlling expression isn't really constant. Instead, this
> > > patch makes it so that a bit field gets used here. Consequently, those
> > > uses where the condition isn't really constant now also need fixing.
> > > 
> > > Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> > > MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> > > if the expression is compile time constant (__builtin_constant_p()
> > > yields true), the array is still deemed of variable length by gcc, and
> > > hence the whole expression doesn't have the intended effect.
> > > 
> > > Signed-off-by: Jan Beulich <jbeulich@novell.com>
> > 
> > We used to use an undefined symbol here; diagnostics are worse but it catches
> > more stuff.
> > 
> > Perhaps a hybrid is the way to go?
> > 
> > #ifndef __OPTIMIZE__
> > #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > #else
> > /* If it's a constant, catch it at compile time, otherwise at link time. */
> > extern int __build_bug_on_failed;
> > #define BUILD_BUG_ON(condition) \
> > 	do { 								\
> > 		((void)sizeof(char[1 - 2*!!(condition)]));		\
> > 		if (condition) __build_bug_on_failed = 1;		\
> > 	} while(0)
> > #endif
> > 
> > Thanks,
> > Rusty.

Re: linux-next: tree build failure

[Hollis Blanchard]

On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> 
> original: built but didn't work
> Jan's: doesn't build
> Rusty's: builds and works
> 
> Where do you want to go from here?

Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
build, and we still need to fix it.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> 
> original: built but didn't work
> Jan's: doesn't build
> Rusty's: builds and works
> 
> Where do you want to go from here?

Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
build, and we still need to fix it.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> 
> original: built but didn't work
> Jan's: doesn't build
> Rusty's: builds and works
> 
> Where do you want to go from here?

Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
build, and we still need to fix it.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
>On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
>> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
>> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
>> 
>> original: built but didn't work
>> Jan's: doesn't build
>> Rusty's: builds and works
>> 
>> Where do you want to go from here?
>
>Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
>build, and we still need to fix it.

My perspective is that it just uncovered already existing brokenness. And
honestly, I won't be able to get to look into this within the next days. (And
btw., when I run into issues with other people's code changes, quite
frequently I'm told to propose a patch, so I'm also having some
philosophical problem understanding why I can't simply expect the same
when people run into issues with changes I made, especially in cases like
this where it wasn't me introducing the broken code.) So, if this can wait
for a couple of days, I can try to find time to look into this. Otherwise, I'd
rely on someone running into the actual issue to implement a solution.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
>On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
>> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
>> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
>> 
>> original: built but didn't work
>> Jan's: doesn't build
>> Rusty's: builds and works
>> 
>> Where do you want to go from here?
>
>Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
>build, and we still need to fix it.

My perspective is that it just uncovered already existing brokenness. And
honestly, I won't be able to get to look into this within the next days. (And
btw., when I run into issues with other people's code changes, quite
frequently I'm told to propose a patch, so I'm also having some
philosophical problem understanding why I can't simply expect the same
when people run into issues with changes I made, especially in cases like
this where it wasn't me introducing the broken code.) So, if this can wait
for a couple of days, I can try to find time to look into this. Otherwise, I'd
rely on someone running into the actual issue to implement a solution.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
>On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
>> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
>> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
>>=20
>> original: built but didn't work
>> Jan's: doesn't build
>> Rusty's: builds and works
>>=20
>> Where do you want to go from here?
>
>Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
>build, and we still need to fix it.

My perspective is that it just uncovered already existing brokenness. And
honestly, I won't be able to get to look into this within the next days. =
(And
btw., when I run into issues with other people's code changes, quite
frequently I'm told to propose a patch, so I'm also having some
philosophical problem understanding why I can't simply expect the same
when people run into issues with changes I made, especially in cases like
this where it wasn't me introducing the broken code.) So, if this can wait
for a couple of days, I can try to find time to look into this. Otherwise, =
I'd
rely on someone running into the actual issue to implement a solution.

Jan

Re: linux-next: tree build failure

[Jan Beulich]

>>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
>On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
>> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
>> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
>> 
>> original: built but didn't work
>> Jan's: doesn't build
>> Rusty's: builds and works
>> 
>> Where do you want to go from here?
>
>Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
>build, and we still need to fix it.

My perspective is that it just uncovered already existing brokenness. And
honestly, I won't be able to get to look into this within the next days. (And
btw., when I run into issues with other people's code changes, quite
frequently I'm told to propose a patch, so I'm also having some
philosophical problem understanding why I can't simply expect the same
when people run into issues with changes I made, especially in cases like
this where it wasn't me introducing the broken code.) So, if this can wait
for a couple of days, I can try to find time to look into this. Otherwise, I'd
rely on someone running into the actual issue to implement a solution.

Jan

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
> >On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> >> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> >> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> >> 
> >> original: built but didn't work
> >> Jan's: doesn't build
> >> Rusty's: builds and works
> >> 
> >> Where do you want to go from here?
> >
> >Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
> >build, and we still need to fix it.
> 
> My perspective is that it just uncovered already existing brokenness. And
> honestly, I won't be able to get to look into this within the next days. (And
> btw., when I run into issues with other people's code changes, quite
> frequently I'm told to propose a patch, so I'm also having some
> philosophical problem understanding why I can't simply expect the same
> when people run into issues with changes I made, especially in cases like
> this where it wasn't me introducing the broken code.) So, if this can wait
> for a couple of days, I can try to find time to look into this. Otherwise, I'd
> rely on someone running into the actual issue to implement a solution.

Sorry, I thought it was clear, but to be more explicit: I propose the
following patch, which replaces the current BUILD_BUG_ON implementation
with Rusty's version.

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -677,18 +677,19 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
-/* Force a compilation error if condition is true, but also produce a
-   result (of value 0 and type size_t), so the expression can be used
-   e.g. in a structure initializer (or where-ever else comma expressions
-   aren't permitted). */
-#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
-#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+/* If it's a constant, catch it at compile time, otherwise at link time. */
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
+#define BUILD_BUG_ON(condition) \
+		do {                                                            \
+				((void)sizeof(char[1 - 2*!!(condition)]));              \
+				if (condition) __build_bug_on_failed = 1;               \
+		} while(0)
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+#endif
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)


-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
> >On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> >> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> >> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> >> 
> >> original: built but didn't work
> >> Jan's: doesn't build
> >> Rusty's: builds and works
> >> 
> >> Where do you want to go from here?
> >
> >Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
> >build, and we still need to fix it.
> 
> My perspective is that it just uncovered already existing brokenness. And
> honestly, I won't be able to get to look into this within the next days. (And
> btw., when I run into issues with other people's code changes, quite
> frequently I'm told to propose a patch, so I'm also having some
> philosophical problem understanding why I can't simply expect the same
> when people run into issues with changes I made, especially in cases like
> this where it wasn't me introducing the broken code.) So, if this can wait
> for a couple of days, I can try to find time to look into this. Otherwise, I'd
> rely on someone running into the actual issue to implement a solution.

Sorry, I thought it was clear, but to be more explicit: I propose the
following patch, which replaces the current BUILD_BUG_ON implementation
with Rusty's version.

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -677,18 +677,19 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
-/* Force a compilation error if condition is true, but also produce a
-   result (of value 0 and type size_t), so the expression can be used
-   e.g. in a structure initializer (or where-ever else comma expressions
-   aren't permitted). */
-#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
-#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+/* If it's a constant, catch it at compile time, otherwise at link time. */
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
+#define BUILD_BUG_ON(condition) \
+		do {                                                            \
+				((void)sizeof(char[1 - 2*!!(condition)]));              \
+				if (condition) __build_bug_on_failed = 1;               \
+		} while(0)
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+#endif
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)


-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard <hollisb@us.ibm.com> 15.10.09 00:57 >>>
> >On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> >> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> >> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> >> 
> >> original: built but didn't work
> >> Jan's: doesn't build
> >> Rusty's: builds and works
> >> 
> >> Where do you want to go from here?
> >
> >Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
> >build, and we still need to fix it.
> 
> My perspective is that it just uncovered already existing brokenness. And
> honestly, I won't be able to get to look into this within the next days. (And
> btw., when I run into issues with other people's code changes, quite
> frequently I'm told to propose a patch, so I'm also having some
> philosophical problem understanding why I can't simply expect the same
> when people run into issues with changes I made, especially in cases like
> this where it wasn't me introducing the broken code.) So, if this can wait
> for a couple of days, I can try to find time to look into this. Otherwise, I'd
> rely on someone running into the actual issue to implement a solution.

Sorry, I thought it was clear, but to be more explicit: I propose the
following patch, which replaces the current BUILD_BUG_ON implementation
with Rusty's version.

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -677,18 +677,19 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
-/* Force a compilation error if condition is true, but also produce a
-   result (of value 0 and type size_t), so the expression can be used
-   e.g. in a structure initializer (or where-ever else comma expressions
-   aren't permitted). */
-#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
-#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+/* If it's a constant, catch it at compile time, otherwise at link time. */
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
+#define BUILD_BUG_ON(condition) \
+		do {                                                            \
+				((void)sizeof(char[1 - 2*!!(condition)]));              \
+				if (condition) __build_bug_on_failed = 1;               \
+		} while(0)
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+#endif
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)


-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Rusty Russell]

On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > My perspective is that it just uncovered already existing brokenness.
> 
> Sorry, I thought it was clear, but to be more explicit: I propose the
> following patch, which replaces the current BUILD_BUG_ON implementation
> with Rusty's version.

OK, I switched my brain back on.  Yeah, I agree: we may still want
BUILD_OR_RUNTIME_BUG_ON one day, but I like this.

It's just missing the giant comment that it needs :)

/**
 * BUILD_BUG_ON - break compile if a condition is true.
 * @cond: the condition which the compiler should know is false.
 *
 * If you have some code which relies on certain constants being equal, or
 * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
 * detect if someone changes it.
 *
 * The implementation uses gcc's reluctance to create a negative array, but
 * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
 * to inline functions).  So as a fallback we use the optimizer; if it can't
 * prove the condition is false, it will cause a link error on the undefined
 * "__build_bug_on_failed".  This error is less neat, and can be harder to
 * track down.
 */

Thanks!
Rusty.

Re: linux-next: tree build failure

[Rusty Russell]

On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > My perspective is that it just uncovered already existing brokenness.
> 
> Sorry, I thought it was clear, but to be more explicit: I propose the
> following patch, which replaces the current BUILD_BUG_ON implementation
> with Rusty's version.

OK, I switched my brain back on.  Yeah, I agree: we may still want
BUILD_OR_RUNTIME_BUG_ON one day, but I like this.

It's just missing the giant comment that it needs :)

/**
 * BUILD_BUG_ON - break compile if a condition is true.
 * @cond: the condition which the compiler should know is false.
 *
 * If you have some code which relies on certain constants being equal, or
 * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
 * detect if someone changes it.
 *
 * The implementation uses gcc's reluctance to create a negative array, but
 * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
 * to inline functions).  So as a fallback we use the optimizer; if it can't
 * prove the condition is false, it will cause a link error on the undefined
 * "__build_bug_on_failed".  This error is less neat, and can be harder to
 * track down.
 */

Thanks!
Rusty.

Re: linux-next: tree build failure

[Rusty Russell]

On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > My perspective is that it just uncovered already existing brokenness.
> 
> Sorry, I thought it was clear, but to be more explicit: I propose the
> following patch, which replaces the current BUILD_BUG_ON implementation
> with Rusty's version.

OK, I switched my brain back on.  Yeah, I agree: we may still want
BUILD_OR_RUNTIME_BUG_ON one day, but I like this.

It's just missing the giant comment that it needs :)

/**
 * BUILD_BUG_ON - break compile if a condition is true.
 * @cond: the condition which the compiler should know is false.
 *
 * If you have some code which relies on certain constants being equal, or
 * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
 * detect if someone changes it.
 *
 * The implementation uses gcc's reluctance to create a negative array, but
 * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
 * to inline functions).  So as a fallback we use the optimizer; if it can't
 * prove the condition is false, it will cause a link error on the undefined
 * "__build_bug_on_failed".  This error is less neat, and can be harder to
 * track down.
 */

Thanks!
Rusty.

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-10-20 at 11:42 +1030, Rusty Russell wrote:
> On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> > On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > > My perspective is that it just uncovered already existing brokenness.
> > 
> > Sorry, I thought it was clear, but to be more explicit: I propose the
> > following patch, which replaces the current BUILD_BUG_ON implementation
> > with Rusty's version.
> 
> OK, I switched my brain back on.  Yeah, I agree: we may still want
> BUILD_OR_RUNTIME_BUG_ON one day, but I like this.
> 
> It's just missing the giant comment that it needs :)
> 
> /**
>  * BUILD_BUG_ON - break compile if a condition is true.
>  * @cond: the condition which the compiler should know is false.
>  *
>  * If you have some code which relies on certain constants being equal, or
>  * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>  * detect if someone changes it.
>  *
>  * The implementation uses gcc's reluctance to create a negative array, but
>  * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>  * to inline functions).  So as a fallback we use the optimizer; if it can't
>  * prove the condition is false, it will cause a link error on the undefined
>  * "__build_bug_on_failed".  This error is less neat, and can be harder to
>  * track down.
>  */

Do you want to put together a signed-off patch Rusty? It's your code, so
I don't feel comfortable doing that.

Once we have that, can we remove the mysterious MAYBE_BUILD_BUG_ON
statements introduced in previous patches? (Does it BUG or doesn't it??)

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-10-20 at 11:42 +1030, Rusty Russell wrote:
> On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> > On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > > My perspective is that it just uncovered already existing brokenness.
> > 
> > Sorry, I thought it was clear, but to be more explicit: I propose the
> > following patch, which replaces the current BUILD_BUG_ON implementation
> > with Rusty's version.
> 
> OK, I switched my brain back on.  Yeah, I agree: we may still want
> BUILD_OR_RUNTIME_BUG_ON one day, but I like this.
> 
> It's just missing the giant comment that it needs :)
> 
> /**
>  * BUILD_BUG_ON - break compile if a condition is true.
>  * @cond: the condition which the compiler should know is false.
>  *
>  * If you have some code which relies on certain constants being equal, or
>  * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>  * detect if someone changes it.
>  *
>  * The implementation uses gcc's reluctance to create a negative array, but
>  * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>  * to inline functions).  So as a fallback we use the optimizer; if it can't
>  * prove the condition is false, it will cause a link error on the undefined
>  * "__build_bug_on_failed".  This error is less neat, and can be harder to
>  * track down.
>  */

Do you want to put together a signed-off patch Rusty? It's your code, so
I don't feel comfortable doing that.

Once we have that, can we remove the mysterious MAYBE_BUILD_BUG_ON
statements introduced in previous patches? (Does it BUG or doesn't it??)

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-10-20 at 11:42 +1030, Rusty Russell wrote:
> On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> > On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > > My perspective is that it just uncovered already existing brokenness.
> > 
> > Sorry, I thought it was clear, but to be more explicit: I propose the
> > following patch, which replaces the current BUILD_BUG_ON implementation
> > with Rusty's version.
> 
> OK, I switched my brain back on.  Yeah, I agree: we may still want
> BUILD_OR_RUNTIME_BUG_ON one day, but I like this.
> 
> It's just missing the giant comment that it needs :)
> 
> /**
>  * BUILD_BUG_ON - break compile if a condition is true.
>  * @cond: the condition which the compiler should know is false.
>  *
>  * If you have some code which relies on certain constants being equal, or
>  * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>  * detect if someone changes it.
>  *
>  * The implementation uses gcc's reluctance to create a negative array, but
>  * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>  * to inline functions).  So as a fallback we use the optimizer; if it can't
>  * prove the condition is false, it will cause a link error on the undefined
>  * "__build_bug_on_failed".  This error is less neat, and can be harder to
>  * track down.
>  */

Do you want to put together a signed-off patch Rusty? It's your code, so
I don't feel comfortable doing that.

Once we have that, can we remove the mysterious MAYBE_BUILD_BUG_ON
statements introduced in previous patches? (Does it BUG or doesn't it??)

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: linux-next: tree build failure

[Hollis Blanchard]

On Tue, 2009-10-20 at 11:42 +1030, Rusty Russell wrote:
> On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote:
> > On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote:
> > > My perspective is that it just uncovered already existing brokenness.
> > 
> > Sorry, I thought it was clear, but to be more explicit: I propose the
> > following patch, which replaces the current BUILD_BUG_ON implementation
> > with Rusty's version.
> 
> OK, I switched my brain back on.  Yeah, I agree: we may still want
> BUILD_OR_RUNTIME_BUG_ON one day, but I like this.
> 
> It's just missing the giant comment that it needs :)
> 
> /**
>  * BUILD_BUG_ON - break compile if a condition is true.
>  * @cond: the condition which the compiler should know is false.
>  *
>  * If you have some code which relies on certain constants being equal, or
>  * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>  * detect if someone changes it.
>  *
>  * The implementation uses gcc's reluctance to create a negative array, but
>  * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>  * to inline functions).  So as a fallback we use the optimizer; if it can't
>  * prove the condition is false, it will cause a link error on the undefined
>  * "__build_bug_on_failed".  This error is less neat, and can be harder to
>  * track down.
>  */

Do you want to put together a signed-off patch Rusty? It's your code, so
I don't feel comfortable doing that.

Once we have that, can we remove the mysterious MAYBE_BUILD_BUG_ON
statements introduced in previous patches? (Does it BUG or doesn't it??)

-- 
Hollis Blanchard
IBM Linux Technology Center

[PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

BUILD_BUG_ON used to use the optimizer to do code elimination or fail
at link time; it was changed to first the size of a negative array (a
nicer compile time error), then (in
8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

bitfields: needs a literal constant at parse time, and can't be put under
	"if (__builtin_constant_p(x))" for example.
negative array: can handle anything, but if the compiler can't tell it's
	a constant, silently has no effect.
link time: breaks link if the compiler can't determine the value, but the
	linker output is not usually as informative as a compiler error.

If we use the negative-array-size method *and* the link time trick,
we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
branches, and maximal ability for the compiler to detect errors at
build time.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -683,12 +683,6 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
 /* Force a compilation error if condition is true, but also produce a
    result (of value 0 and type size_t), so the expression can be used
    e.g. in a structure initializer (or where-ever else comma expressions
@@ -696,6 +690,33 @@ struct sysinfo {
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @cond: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions).  So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * "__build_bug_on_failed".  This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)					\
+	do {							\
+		((void)sizeof(char[1 - 2*!!(condition)]));	\
+		if (condition) __build_bug_on_failed = 1;	\
+	} while(0)
+#endif
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 

[PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

BUILD_BUG_ON used to use the optimizer to do code elimination or fail
at link time; it was changed to first the size of a negative array (a
nicer compile time error), then (in
8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

bitfields: needs a literal constant at parse time, and can't be put under
	"if (__builtin_constant_p(x))" for example.
negative array: can handle anything, but if the compiler can't tell it's
	a constant, silently has no effect.
link time: breaks link if the compiler can't determine the value, but the
	linker output is not usually as informative as a compiler error.

If we use the negative-array-size method *and* the link time trick,
we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
branches, and maximal ability for the compiler to detect errors at
build time.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -683,12 +683,6 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
 /* Force a compilation error if condition is true, but also produce a
    result (of value 0 and type size_t), so the expression can be used
    e.g. in a structure initializer (or where-ever else comma expressions
@@ -696,6 +690,33 @@ struct sysinfo {
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @cond: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions).  So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * "__build_bug_on_failed".  This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)					\
+	do {							\
+		((void)sizeof(char[1 - 2*!!(condition)]));	\
+		if (condition) __build_bug_on_failed = 1;	\
+	} while(0)
+#endif
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 

[PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

BUILD_BUG_ON used to use the optimizer to do code elimination or fail
at link time; it was changed to first the size of a negative array (a
nicer compile time error), then (in
8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

bitfields: needs a literal constant at parse time, and can't be put under
	"if (__builtin_constant_p(x))" for example.
negative array: can handle anything, but if the compiler can't tell it's
	a constant, silently has no effect.
link time: breaks link if the compiler can't determine the value, but the
	linker output is not usually as informative as a compiler error.

If we use the negative-array-size method *and* the link time trick,
we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
branches, and maximal ability for the compiler to detect errors at
build time.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -683,12 +683,6 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
 /* Force a compilation error if condition is true, but also produce a
    result (of value 0 and type size_t), so the expression can be used
    e.g. in a structure initializer (or where-ever else comma expressions
@@ -696,6 +690,33 @@ struct sysinfo {
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @cond: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions).  So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * "__build_bug_on_failed".  This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)					\
+	do {							\
+		((void)sizeof(char[1 - 2*!!(condition)]));	\
+		if (condition) __build_bug_on_failed = 1;	\
+	} while(0)
+#endif
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 

[PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

BUILD_BUG_ON used to use the optimizer to do code elimination or fail
at link time; it was changed to first the size of a negative array (a
nicer compile time error), then (in
8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

bitfields: needs a literal constant at parse time, and can't be put under
	"if (__builtin_constant_p(x))" for example.
negative array: can handle anything, but if the compiler can't tell it's
	a constant, silently has no effect.
link time: breaks link if the compiler can't determine the value, but the
	linker output is not usually as informative as a compiler error.

If we use the negative-array-size method *and* the link time trick,
we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
branches, and maximal ability for the compiler to detect errors at
build time.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -683,12 +683,6 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
 /* Force a compilation error if condition is true, but also produce a
    result (of value 0 and type size_t), so the expression can be used
    e.g. in a structure initializer (or where-ever else comma expressions
@@ -696,6 +690,33 @@ struct sysinfo {
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @cond: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions).  So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * "__build_bug_on_failed".  This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)					\
+	do {							\
+		((void)sizeof(char[1 - 2*!!(condition)]));	\
+		if (condition) __build_bug_on_failed = 1;	\
+	} while(0)
+#endif
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>at link time; it was changed to first the size of a negative array (a
>nicer compile time error), then (in
>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>
>bitfields: needs a literal constant at parse time, and can't be put under
>	"if (__builtin_constant_p(x))" for example.
>negative array: can handle anything, but if the compiler can't tell it's
>	a constant, silently has no effect.
>link time: breaks link if the compiler can't determine the value, but the
>	linker output is not usually as informative as a compiler error.
>
>If we use the negative-array-size method *and* the link time trick,
>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>branches, and maximal ability for the compiler to detect errors at
>build time.
>
>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>--- a/include/linux/kernel.h
>+++ b/include/linux/kernel.h
>@@ -683,12 +683,6 @@ struct sysinfo {
> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
> };
> 
>-/* Force a compilation error if condition is true */
>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>-
>-/* Force a compilation error if condition is constant and true */
>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>-
> /* Force a compilation error if condition is true, but also produce a
>    result (of value 0 and type size_t), so the expression can be used
>    e.g. in a structure initializer (or where-ever else comma expressions
>@@ -696,6 +690,33 @@ struct sysinfo {
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
>+/**
>+ * BUILD_BUG_ON - break compile if a condition is true.
>+ * @cond: the condition which the compiler should know is false.
>+ *
>+ * If you have some code which relies on certain constants being equal, or
>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>+ * detect if someone changes it.
>+ *
>+ * The implementation uses gcc's reluctance to create a negative array, but
>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>+ * to inline functions).  So as a fallback we use the optimizer; if it can't
>+ * prove the condition is false, it will cause a link error on the undefined
>+ * "__build_bug_on_failed".  This error message can be harder to track down
>+ * though, hence the two different methods.
>+ */
>+#ifndef __OPTIMIZE__
>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>+#else
>+extern int __build_bug_on_failed;

Hmm, what exactly is __build_bug_on_failed?

>+#define BUILD_BUG_ON(condition)					\
>+	do {							\
>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>+		if (condition) __build_bug_on_failed = 1;	\
>+	} while(0)
>+#endif
>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>+
> /* Trap pasters of __FUNCTION__ at compile-time */
> #define __FUNCTION__ (__func__)
> 
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
Live like a child, think like the god.
 

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>at link time; it was changed to first the size of a negative array (a
>nicer compile time error), then (in
>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>
>bitfields: needs a literal constant at parse time, and can't be put under
>	"if (__builtin_constant_p(x))" for example.
>negative array: can handle anything, but if the compiler can't tell it's
>	a constant, silently has no effect.
>link time: breaks link if the compiler can't determine the value, but the
>	linker output is not usually as informative as a compiler error.
>
>If we use the negative-array-size method *and* the link time trick,
>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>branches, and maximal ability for the compiler to detect errors at
>build time.
>
>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>--- a/include/linux/kernel.h
>+++ b/include/linux/kernel.h
>@@ -683,12 +683,6 @@ struct sysinfo {
> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
> };
> 
>-/* Force a compilation error if condition is true */
>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>-
>-/* Force a compilation error if condition is constant and true */
>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>-
> /* Force a compilation error if condition is true, but also produce a
>    result (of value 0 and type size_t), so the expression can be used
>    e.g. in a structure initializer (or where-ever else comma expressions
>@@ -696,6 +690,33 @@ struct sysinfo {
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
>+/**
>+ * BUILD_BUG_ON - break compile if a condition is true.
>+ * @cond: the condition which the compiler should know is false.
>+ *
>+ * If you have some code which relies on certain constants being equal, or
>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>+ * detect if someone changes it.
>+ *
>+ * The implementation uses gcc's reluctance to create a negative array, but
>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>+ * to inline functions).  So as a fallback we use the optimizer; if it can't
>+ * prove the condition is false, it will cause a link error on the undefined
>+ * "__build_bug_on_failed".  This error message can be harder to track down
>+ * though, hence the two different methods.
>+ */
>+#ifndef __OPTIMIZE__
>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>+#else
>+extern int __build_bug_on_failed;

Hmm, what exactly is __build_bug_on_failed?

>+#define BUILD_BUG_ON(condition)					\
>+	do {							\
>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>+		if (condition) __build_bug_on_failed = 1;	\
>+	} while(0)
>+#endif
>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>+
> /* Trap pasters of __FUNCTION__ at compile-time */
> #define __FUNCTION__ (__func__)
> 
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
Live like a child, think like the god.
 

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>at link time; it was changed to first the size of a negative array (a
>nicer compile time error), then (in
>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>
>bitfields: needs a literal constant at parse time, and can't be put under
>	"if (__builtin_constant_p(x))" for example.
>negative array: can handle anything, but if the compiler can't tell it's
>	a constant, silently has no effect.
>link time: breaks link if the compiler can't determine the value, but the
>	linker output is not usually as informative as a compiler error.
>
>If we use the negative-array-size method *and* the link time trick,
>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>branches, and maximal ability for the compiler to detect errors at
>build time.
>
>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>--- a/include/linux/kernel.h
>+++ b/include/linux/kernel.h
>@@ -683,12 +683,6 @@ struct sysinfo {
> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
> };
> 
>-/* Force a compilation error if condition is true */
>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>-
>-/* Force a compilation error if condition is constant and true */
>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>-
> /* Force a compilation error if condition is true, but also produce a
>    result (of value 0 and type size_t), so the expression can be used
>    e.g. in a structure initializer (or where-ever else comma expressions
>@@ -696,6 +690,33 @@ struct sysinfo {
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
>+/**
>+ * BUILD_BUG_ON - break compile if a condition is true.
>+ * @cond: the condition which the compiler should know is false.
>+ *
>+ * If you have some code which relies on certain constants being equal, or
>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>+ * detect if someone changes it.
>+ *
>+ * The implementation uses gcc's reluctance to create a negative array, but
>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
>+ * to inline functions).  So as a fallback we use the optimizer; if it can't
>+ * prove the condition is false, it will cause a link error on the undefined
>+ * "__build_bug_on_failed".  This error message can be harder to track down
>+ * though, hence the two different methods.
>+ */
>+#ifndef __OPTIMIZE__
>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>+#else
>+extern int __build_bug_on_failed;

Hmm, what exactly is __build_bug_on_failed?

>+#define BUILD_BUG_ON(condition)					\
>+	do {							\
>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>+		if (condition) __build_bug_on_failed = 1;	\
>+	} while(0)
>+#endif
>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>+
> /* Trap pasters of __FUNCTION__ at compile-time */
> #define __FUNCTION__ (__func__)
> 
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
Live like a child, think like the god.
 

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Alan Jenkins]

On 10/20/09, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>at link time; it was changed to first the size of a negative array (a
>>nicer compile time error), then (in
>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>
>>bitfields: needs a literal constant at parse time, and can't be put under
>>	"if (__builtin_constant_p(x))" for example.
>>negative array: can handle anything, but if the compiler can't tell it's
>>	a constant, silently has no effect.
>>link time: breaks link if the compiler can't determine the value, but the
>>	linker output is not usually as informative as a compiler error.
>>
>>If we use the negative-array-size method *and* the link time trick,
>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>branches, and maximal ability for the compiler to detect errors at
>>build time.
>>
>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>
>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>--- a/include/linux/kernel.h
>>+++ b/include/linux/kernel.h
>>@@ -683,12 +683,6 @@ struct sysinfo {
>> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>> };
>>
>>-/* Force a compilation error if condition is true */
>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>-
>>-/* Force a compilation error if condition is constant and true */
>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>-
>> /* Force a compilation error if condition is true, but also produce a
>>    result (of value 0 and type size_t), so the expression can be used
>>    e.g. in a structure initializer (or where-ever else comma expressions
>>@@ -696,6 +690,33 @@ struct sysinfo {
>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>
>>+/**
>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>+ * @cond: the condition which the compiler should know is false.
>>+ *
>>+ * If you have some code which relies on certain constants being equal, or
>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>>+ * detect if someone changes it.
>>+ *
>>+ * The implementation uses gcc's reluctance to create a negative array,
>> but
>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>> arguments
>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>> can't
>>+ * prove the condition is false, it will cause a link error on the
>> undefined
>>+ * "__build_bug_on_failed".  This error message can be harder to track
>> down
>>+ * though, hence the two different methods.
>>+ */
>>+#ifndef __OPTIMIZE__
>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>>+#else
>>+extern int __build_bug_on_failed;
>
> Hmm, what exactly is __build_bug_on_failed?

Well, we haven't added a definition for it in this patch.  I'm sure
grep will tell you it wasn't defined before hand either.  So any
reference to it is an error - which will be reported at link time.

>>+#define BUILD_BUG_ON(condition)					\
>>+	do {							\
>>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>>+		if (condition) __build_bug_on_failed = 1;	\

If "condition" is known false at compile time, gcc -O will eliminate
the code which refers to __build_bug_on_failed.  If it's not proved to
be false - it will break the build, which is exactly what we want
BUILD_BUG_ON to do.

>>+	} while(0)
>>+#endif
>>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>>+
>> /* Trap pasters of __FUNCTION__ at compile-time */
>> #define __FUNCTION__ (__func__)
>>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Alan Jenkins]

On 10/20/09, Am=E9rico Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>at link time; it was changed to first the size of a negative array (a
>>nicer compile time error), then (in
>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>
>>bitfields: needs a literal constant at parse time, and can't be put under
>>	"if (__builtin_constant_p(x))" for example.
>>negative array: can handle anything, but if the compiler can't tell it's
>>	a constant, silently has no effect.
>>link time: breaks link if the compiler can't determine the value, but the
>>	linker output is not usually as informative as a compiler error.
>>
>>If we use the negative-array-size method *and* the link time trick,
>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>branches, and maximal ability for the compiler to detect errors at
>>build time.
>>
>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>
>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>--- a/include/linux/kernel.h
>>+++ b/include/linux/kernel.h
>>@@ -683,12 +683,6 @@ struct sysinfo {
>> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. *=
/
>> };
>>
>>-/* Force a compilation error if condition is true */
>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>-
>>-/* Force a compilation error if condition is constant and true */
>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>-
>> /* Force a compilation error if condition is true, but also produce a
>>    result (of value 0 and type size_t), so the expression can be used
>>    e.g. in a structure initializer (or where-ever else comma expressions
>>@@ -696,6 +690,33 @@ struct sysinfo {
>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>
>>+/**
>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>+ * @cond: the condition which the compiler should know is false.
>>+ *
>>+ * If you have some code which relies on certain constants being equal, =
or
>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON t=
o
>>+ * detect if someone changes it.
>>+ *
>>+ * The implementation uses gcc's reluctance to create a negative array,
>> but
>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>> arguments
>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>> can't
>>+ * prove the condition is false, it will cause a link error on the
>> undefined
>>+ * "__build_bug_on_failed".  This error message can be harder to track
>> down
>>+ * though, hence the two different methods.
>>+ */
>>+#ifndef __OPTIMIZE__
>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])=
)
>>+#else
>>+extern int __build_bug_on_failed;
>
> Hmm, what exactly is __build_bug_on_failed?

Well, we haven't added a definition for it in this patch.  I'm sure
grep will tell you it wasn't defined before hand either.  So any
reference to it is an error - which will be reported at link time.

>>+#define BUILD_BUG_ON(condition)					\
>>+	do {							\
>>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>>+		if (condition) __build_bug_on_failed =3D 1;	\

If "condition" is known false at compile time, gcc -O will eliminate
the code which refers to __build_bug_on_failed.  If it's not proved to
be false - it will break the build, which is exactly what we want
BUILD_BUG_ON to do.

>>+	} while(0)
>>+#endif
>>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>>+
>> /* Trap pasters of __FUNCTION__ at compile-time */
>> #define __FUNCTION__ (__func__)
>>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Alan Jenkins]

On 10/20/09, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>at link time; it was changed to first the size of a negative array (a
>>nicer compile time error), then (in
>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>
>>bitfields: needs a literal constant at parse time, and can't be put under
>>	"if (__builtin_constant_p(x))" for example.
>>negative array: can handle anything, but if the compiler can't tell it's
>>	a constant, silently has no effect.
>>link time: breaks link if the compiler can't determine the value, but the
>>	linker output is not usually as informative as a compiler error.
>>
>>If we use the negative-array-size method *and* the link time trick,
>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>branches, and maximal ability for the compiler to detect errors at
>>build time.
>>
>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>
>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>--- a/include/linux/kernel.h
>>+++ b/include/linux/kernel.h
>>@@ -683,12 +683,6 @@ struct sysinfo {
>> 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>> };
>>
>>-/* Force a compilation error if condition is true */
>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>-
>>-/* Force a compilation error if condition is constant and true */
>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>-
>> /* Force a compilation error if condition is true, but also produce a
>>    result (of value 0 and type size_t), so the expression can be used
>>    e.g. in a structure initializer (or where-ever else comma expressions
>>@@ -696,6 +690,33 @@ struct sysinfo {
>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>
>>+/**
>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>+ * @cond: the condition which the compiler should know is false.
>>+ *
>>+ * If you have some code which relies on certain constants being equal, or
>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>>+ * detect if someone changes it.
>>+ *
>>+ * The implementation uses gcc's reluctance to create a negative array,
>> but
>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>> arguments
>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>> can't
>>+ * prove the condition is false, it will cause a link error on the
>> undefined
>>+ * "__build_bug_on_failed".  This error message can be harder to track
>> down
>>+ * though, hence the two different methods.
>>+ */
>>+#ifndef __OPTIMIZE__
>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>>+#else
>>+extern int __build_bug_on_failed;
>
> Hmm, what exactly is __build_bug_on_failed?

Well, we haven't added a definition for it in this patch.  I'm sure
grep will tell you it wasn't defined before hand either.  So any
reference to it is an error - which will be reported at link time.

>>+#define BUILD_BUG_ON(condition)					\
>>+	do {							\
>>+		((void)sizeof(char[1 - 2*!!(condition)]));	\
>>+		if (condition) __build_bug_on_failed = 1;	\

If "condition" is known false at compile time, gcc -O will eliminate
the code which refers to __build_bug_on_failed.  If it's not proved to
be false - it will break the build, which is exactly what we want
BUILD_BUG_ON to do.

>>+	} while(0)
>>+#endif
>>+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
>>+
>> /* Trap pasters of __FUNCTION__ at compile-time */
>> #define __FUNCTION__ (__func__)
>>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks Rusty, this indeed fixes the problem.

Acked-by: Hollis Blanchard <hollisb@us.ibm.com>

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

Thanks Rusty, this indeed fixes the problem.

Acked-by: Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks Rusty, this indeed fixes the problem.

Acked-by: Hollis Blanchard <hollisb@us.ibm.com>

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks Rusty, this indeed fixes the problem.

Acked-by: Hollis Blanchard <hollisb@us.ibm.com>

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 10:43 PM, Alan Jenkins
<sourcejedi.lkml@googlemail.com> wrote:
> On 10/20/09, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>>at link time; it was changed to first the size of a negative array (a
>>>nicer compile time error), then (in
>>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>>
>>>bitfields: needs a literal constant at parse time, and can't be put under
>>>      "if (__builtin_constant_p(x))" for example.
>>>negative array: can handle anything, but if the compiler can't tell it's
>>>      a constant, silently has no effect.
>>>link time: breaks link if the compiler can't determine the value, but the
>>>      linker output is not usually as informative as a compiler error.
>>>
>>>If we use the negative-array-size method *and* the link time trick,
>>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>>branches, and maximal ability for the compiler to detect errors at
>>>build time.
>>>
>>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>>
>>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>>--- a/include/linux/kernel.h
>>>+++ b/include/linux/kernel.h
>>>@@ -683,12 +683,6 @@ struct sysinfo {
>>>      char _f[20-2*sizeof(long)-sizeof(int)]; /* Padding: libc5 uses this.. */
>>> };
>>>
>>>-/* Force a compilation error if condition is true */
>>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>>-
>>>-/* Force a compilation error if condition is constant and true */
>>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>>-
>>> /* Force a compilation error if condition is true, but also produce a
>>>    result (of value 0 and type size_t), so the expression can be used
>>>    e.g. in a structure initializer (or where-ever else comma expressions
>>>@@ -696,6 +690,33 @@ struct sysinfo {
>>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>>
>>>+/**
>>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>>+ * @cond: the condition which the compiler should know is false.
>>>+ *
>>>+ * If you have some code which relies on certain constants being equal, or
>>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>>>+ * detect if someone changes it.
>>>+ *
>>>+ * The implementation uses gcc's reluctance to create a negative array,
>>> but
>>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>>> arguments
>>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>>> can't
>>>+ * prove the condition is false, it will cause a link error on the
>>> undefined
>>>+ * "__build_bug_on_failed".  This error message can be harder to track
>>> down
>>>+ * though, hence the two different methods.
>>>+ */
>>>+#ifndef __OPTIMIZE__
>>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>>>+#else
>>>+extern int __build_bug_on_failed;
>>
>> Hmm, what exactly is __build_bug_on_failed?
>
> Well, we haven't added a definition for it in this patch.  I'm sure
> grep will tell you it wasn't defined before hand either.  So any
> reference to it is an error - which will be reported at link time.
>
>>>+#define BUILD_BUG_ON(condition)                                      \
>>>+     do {                                                    \
>>>+             ((void)sizeof(char[1 - 2*!!(condition)]));      \
>>>+             if (condition) __build_bug_on_failed = 1;       \
>
> If "condition" is known false at compile time, gcc -O will eliminate
> the code which refers to __build_bug_on_failed.  If it's not proved to
> be false - it will break the build, which is exactly what we want
> BUILD_BUG_ON to do.

Ah, clever trick! Got it.
Thanks!

Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 10:43 PM, Alan Jenkins
<sourcejedi.lkml-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> On 10/20/09, Américo Wang <xiyou.wangcong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>>at link time; it was changed to first the size of a negative array (a
>>>nicer compile time error), then (in
>>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>>
>>>bitfields: needs a literal constant at parse time, and can't be put under
>>>      "if (__builtin_constant_p(x))" for example.
>>>negative array: can handle anything, but if the compiler can't tell it's
>>>      a constant, silently has no effect.
>>>link time: breaks link if the compiler can't determine the value, but the
>>>      linker output is not usually as informative as a compiler error.
>>>
>>>If we use the negative-array-size method *and* the link time trick,
>>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>>branches, and maximal ability for the compiler to detect errors at
>>>build time.
>>>
>>>Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
>>>
>>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>>--- a/include/linux/kernel.h
>>>+++ b/include/linux/kernel.h
>>>@@ -683,12 +683,6 @@ struct sysinfo {
>>>      char _f[20-2*sizeof(long)-sizeof(int)]; /* Padding: libc5 uses this.. */
>>> };
>>>
>>>-/* Force a compilation error if condition is true */
>>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>>-
>>>-/* Force a compilation error if condition is constant and true */
>>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>>-
>>> /* Force a compilation error if condition is true, but also produce a
>>>    result (of value 0 and type size_t), so the expression can be used
>>>    e.g. in a structure initializer (or where-ever else comma expressions
>>>@@ -696,6 +690,33 @@ struct sysinfo {
>>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>>
>>>+/**
>>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>>+ * @cond: the condition which the compiler should know is false.
>>>+ *
>>>+ * If you have some code which relies on certain constants being equal, or
>>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>>>+ * detect if someone changes it.
>>>+ *
>>>+ * The implementation uses gcc's reluctance to create a negative array,
>>> but
>>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>>> arguments
>>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>>> can't
>>>+ * prove the condition is false, it will cause a link error on the
>>> undefined
>>>+ * "__build_bug_on_failed".  This error message can be harder to track
>>> down
>>>+ * though, hence the two different methods.
>>>+ */
>>>+#ifndef __OPTIMIZE__
>>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>>>+#else
>>>+extern int __build_bug_on_failed;
>>
>> Hmm, what exactly is __build_bug_on_failed?
>
> Well, we haven't added a definition for it in this patch.  I'm sure
> grep will tell you it wasn't defined before hand either.  So any
> reference to it is an error - which will be reported at link time.
>
>>>+#define BUILD_BUG_ON(condition)                                      \
>>>+     do {                                                    \
>>>+             ((void)sizeof(char[1 - 2*!!(condition)]));      \
>>>+             if (condition) __build_bug_on_failed = 1;       \
>
> If "condition" is known false at compile time, gcc -O will eliminate
> the code which refers to __build_bug_on_failed.  If it's not proved to
> be false - it will break the build, which is exactly what we want
> BUILD_BUG_ON to do.

Ah, clever trick! Got it.
Thanks!

Reviewed-by: WANG Cong <xiyou.wangcong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 10:43 PM, Alan Jenkins
<sourcejedi.lkml@googlemail.com> wrote:
> On 10/20/09, Am=C3=A9rico Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>>at link time; it was changed to first the size of a negative array (a
>>>nicer compile time error), then (in
>>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>>
>>>bitfields: needs a literal constant at parse time, and can't be put unde=
r
>>> =C2=A0 =C2=A0 =C2=A0"if (__builtin_constant_p(x))" for example.
>>>negative array: can handle anything, but if the compiler can't tell it's
>>> =C2=A0 =C2=A0 =C2=A0a constant, silently has no effect.
>>>link time: breaks link if the compiler can't determine the value, but th=
e
>>> =C2=A0 =C2=A0 =C2=A0linker output is not usually as informative as a co=
mpiler error.
>>>
>>>If we use the negative-array-size method *and* the link time trick,
>>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>>branches, and maximal ability for the compiler to detect errors at
>>>build time.
>>>
>>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>>
>>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>>--- a/include/linux/kernel.h
>>>+++ b/include/linux/kernel.h
>>>@@ -683,12 +683,6 @@ struct sysinfo {
>>> =C2=A0 =C2=A0 =C2=A0char _f[20-2*sizeof(long)-sizeof(int)]; /* Padding:=
 libc5 uses this.. */
>>> };
>>>
>>>-/* Force a compilation error if condition is true */
>>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>>-
>>>-/* Force a compilation error if condition is constant and true */
>>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>>-
>>> /* Force a compilation error if condition is true, but also produce a
>>> =C2=A0 =C2=A0result (of value 0 and type size_t), so the expression can=
 be used
>>> =C2=A0 =C2=A0e.g. in a structure initializer (or where-ever else comma =
expressions
>>>@@ -696,6 +690,33 @@ struct sysinfo {
>>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>>
>>>+/**
>>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>>+ * @cond: the condition which the compiler should know is false.
>>>+ *
>>>+ * If you have some code which relies on certain constants being equal,=
 or
>>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON =
to
>>>+ * detect if someone changes it.
>>>+ *
>>>+ * The implementation uses gcc's reluctance to create a negative array,
>>> but
>>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>>> arguments
>>>+ * to inline functions). =C2=A0So as a fallback we use the optimizer; i=
f it
>>> can't
>>>+ * prove the condition is false, it will cause a link error on the
>>> undefined
>>>+ * "__build_bug_on_failed". =C2=A0This error message can be harder to t=
rack
>>> down
>>>+ * though, hence the two different methods.
>>>+ */
>>>+#ifndef __OPTIMIZE__
>>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]=
))
>>>+#else
>>>+extern int __build_bug_on_failed;
>>
>> Hmm, what exactly is __build_bug_on_failed?
>
> Well, we haven't added a definition for it in this patch. =C2=A0I'm sure
> grep will tell you it wasn't defined before hand either. =C2=A0So any
> reference to it is an error - which will be reported at link time.
>
>>>+#define BUILD_BUG_ON(condition) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0\
>>>+ =C2=A0 =C2=A0 do { =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\
>>>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ((void)sizeof(char[1 - 2*!!(=
condition)])); =C2=A0 =C2=A0 =C2=A0\
>>>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (condition) __build_bug_o=
n_failed =3D 1; =C2=A0 =C2=A0 =C2=A0 \
>
> If "condition" is known false at compile time, gcc -O will eliminate
> the code which refers to __build_bug_on_failed. =C2=A0If it's not proved =
to
> be false - it will break the build, which is exactly what we want
> BUILD_BUG_ON to do.

Ah, clever trick! Got it.
Thanks!

Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Américo Wang]

On Tue, Oct 20, 2009 at 10:43 PM, Alan Jenkins
<sourcejedi.lkml@googlemail.com> wrote:
> On 10/20/09, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Oct 20, 2009 at 02:15:33PM +1030, Rusty Russell wrote:
>>>BUILD_BUG_ON used to use the optimizer to do code elimination or fail
>>>at link time; it was changed to first the size of a negative array (a
>>>nicer compile time error), then (in
>>>8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
>>>
>>>bitfields: needs a literal constant at parse time, and can't be put under
>>>      "if (__builtin_constant_p(x))" for example.
>>>negative array: can handle anything, but if the compiler can't tell it's
>>>      a constant, silently has no effect.
>>>link time: breaks link if the compiler can't determine the value, but the
>>>      linker output is not usually as informative as a compiler error.
>>>
>>>If we use the negative-array-size method *and* the link time trick,
>>>we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
>>>branches, and maximal ability for the compiler to detect errors at
>>>build time.
>>>
>>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>>
>>>diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>>--- a/include/linux/kernel.h
>>>+++ b/include/linux/kernel.h
>>>@@ -683,12 +683,6 @@ struct sysinfo {
>>>      char _f[20-2*sizeof(long)-sizeof(int)]; /* Padding: libc5 uses this.. */
>>> };
>>>
>>>-/* Force a compilation error if condition is true */
>>>-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
>>>-
>>>-/* Force a compilation error if condition is constant and true */
>>>-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
>>>-
>>> /* Force a compilation error if condition is true, but also produce a
>>>    result (of value 0 and type size_t), so the expression can be used
>>>    e.g. in a structure initializer (or where-ever else comma expressions
>>>@@ -696,6 +690,33 @@ struct sysinfo {
>>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>>> #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>>>
>>>+/**
>>>+ * BUILD_BUG_ON - break compile if a condition is true.
>>>+ * @cond: the condition which the compiler should know is false.
>>>+ *
>>>+ * If you have some code which relies on certain constants being equal, or
>>>+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
>>>+ * detect if someone changes it.
>>>+ *
>>>+ * The implementation uses gcc's reluctance to create a negative array,
>>> but
>>>+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not
>>> arguments
>>>+ * to inline functions).  So as a fallback we use the optimizer; if it
>>> can't
>>>+ * prove the condition is false, it will cause a link error on the
>>> undefined
>>>+ * "__build_bug_on_failed".  This error message can be harder to track
>>> down
>>>+ * though, hence the two different methods.
>>>+ */
>>>+#ifndef __OPTIMIZE__
>>>+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>>>+#else
>>>+extern int __build_bug_on_failed;
>>
>> Hmm, what exactly is __build_bug_on_failed?
>
> Well, we haven't added a definition for it in this patch.  I'm sure
> grep will tell you it wasn't defined before hand either.  So any
> reference to it is an error - which will be reported at link time.
>
>>>+#define BUILD_BUG_ON(condition)                                      \
>>>+     do {                                                    \
>>>+             ((void)sizeof(char[1 - 2*!!(condition)]));      \
>>>+             if (condition) __build_bug_on_failed = 1;       \
>
> If "condition" is known false at compile time, gcc -O will eliminate
> the code which refers to __build_bug_on_failed.  If it's not proved to
> be false - it will break the build, which is exactly what we want
> BUILD_BUG_ON to do.

Ah, clever trick! Got it.
Thanks!

Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
> 
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)

What's the state of this patch?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
> 
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)

What's the state of this patch?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Hollis Blanchard]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
> 
> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
> 
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
> 
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)

What's the state of this patch?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +

I decided to try this in linux-next, but an x86_64 allmodconfig build
gave this (gcc 4.4.0):

ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!

I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
virtio_has_feature() (in include/linux/virtio_config.h) which is called
all over the place.  Unfortunately, virtio_has_feature() gets uninlined
in those two files ...

I have taken the patch back out again for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org> wrote:
>
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +

I decided to try this in linux-next, but an x86_64 allmodconfig build
gave this (gcc 4.4.0):

ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!

I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
virtio_has_feature() (in include/linux/virtio_config.h) which is called
all over the place.  Unfortunately, virtio_has_feature() gets uninlined
in those two files ...

I have taken the patch back out again for today.
-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +

I decided to try this in linux-next, but an x86_64 allmodconfig build
gave this (gcc 4.4.0):

ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!

I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
virtio_has_feature() (in include/linux/virtio_config.h) which is called
all over the place.  Unfortunately, virtio_has_feature() gets uninlined
in those two files ...

I have taken the patch back out again for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +

I decided to try this in linux-next, but an x86_64 allmodconfig build
gave this (gcc 4.4.0):

ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!

I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
virtio_has_feature() (in include/linux/virtio_config.h) which is called
all over the place.  Unfortunately, virtio_has_feature() gets uninlined
in those two files ...

I have taken the patch back out again for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Benjamin Herrenschmidt]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

What's the status with this patch ? The lack of it breaks my KVM stuff
in powerpc...

Cheers,
Ben.

> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
>  
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>  
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Benjamin Herrenschmidt]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

What's the status with this patch ? The lack of it breaks my KVM stuff
in powerpc...

Cheers,
Ben.

> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
>  
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>  
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Benjamin Herrenschmidt]

On Tue, 2009-10-20 at 14:15 +1030, Rusty Russell wrote:
> BUILD_BUG_ON used to use the optimizer to do code elimination or fail
> at link time; it was changed to first the size of a negative array (a
> nicer compile time error), then (in
> 8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.

What's the status with this patch ? The lack of it breaks my KVM stuff
in powerpc...

Cheers,
Ben.

> bitfields: needs a literal constant at parse time, and can't be put under
> 	"if (__builtin_constant_p(x))" for example.
> negative array: can handle anything, but if the compiler can't tell it's
> 	a constant, silently has no effect.
> link time: breaks link if the compiler can't determine the value, but the
> 	linker output is not usually as informative as a compiler error.
> 
> If we use the negative-array-size method *and* the link time trick,
> we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
> branches, and maximal ability for the compiler to detect errors at
> build time.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -683,12 +683,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
>  
> -/* Force a compilation error if condition is true */
> -#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
> -
> -/* Force a compilation error if condition is constant and true */
> -#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
> -
>  /* Force a compilation error if condition is true, but also produce a
>     result (of value 0 and type size_t), so the expression can be used
>     e.g. in a structure initializer (or where-ever else comma expressions
> @@ -696,6 +690,33 @@ struct sysinfo {
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
>  
> +/**
> + * BUILD_BUG_ON - break compile if a condition is true.
> + * @cond: the condition which the compiler should know is false.
> + *
> + * If you have some code which relies on certain constants being equal, or
> + * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * detect if someone changes it.
> + *
> + * The implementation uses gcc's reluctance to create a negative array, but
> + * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> + * to inline functions).  So as a fallback we use the optimizer; if it can't
> + * prove the condition is false, it will cause a link error on the undefined
> + * "__build_bug_on_failed".  This error message can be harder to track down
> + * though, hence the two different methods.
> + */
> +#ifndef __OPTIMIZE__
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#else
> +extern int __build_bug_on_failed;
> +#define BUILD_BUG_ON(condition)					\
> +	do {							\
> +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> +		if (condition) __build_bug_on_failed = 1;	\
> +	} while(0)
> +#endif
> +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> +
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 10:50:20 am Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > +#ifndef __OPTIMIZE__
> > +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > +#else
> > +extern int __build_bug_on_failed;
> > +#define BUILD_BUG_ON(condition)					\
> > +	do {							\
> > +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> > +		if (condition) __build_bug_on_failed = 1;	\
> > +	} while(0)
> > +#endif
> > +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> > +
> 
> I decided to try this in linux-next, but an x86_64 allmodconfig build
> gave this (gcc 4.4.0):
> 
> ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
> ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!
> 
> I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
> virtio_has_feature() (in include/linux/virtio_config.h) which is called
> all over the place.  Unfortunately, virtio_has_feature() gets uninlined
> in those two files ...

Huh?  virtio_has_feature does:

	if (__builtin_constant_p(fbit))
		BUILD_BUG_ON(fbit >= 32);
	else
		BUG_ON(fbit >= 32);

So, if it's not a constant, gcc should throw away that first branch.  If it
is, it should optimize it away (and no, these are not real bugs AFAICT).

I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
was fine.  Will try 4.4.0 now.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 10:50:20 am Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > +#ifndef __OPTIMIZE__
> > +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > +#else
> > +extern int __build_bug_on_failed;
> > +#define BUILD_BUG_ON(condition)					\
> > +	do {							\
> > +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> > +		if (condition) __build_bug_on_failed = 1;	\
> > +	} while(0)
> > +#endif
> > +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> > +
> 
> I decided to try this in linux-next, but an x86_64 allmodconfig build
> gave this (gcc 4.4.0):
> 
> ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
> ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!
> 
> I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
> virtio_has_feature() (in include/linux/virtio_config.h) which is called
> all over the place.  Unfortunately, virtio_has_feature() gets uninlined
> in those two files ...

Huh?  virtio_has_feature does:

	if (__builtin_constant_p(fbit))
		BUILD_BUG_ON(fbit >= 32);
	else
		BUG_ON(fbit >= 32);

So, if it's not a constant, gcc should throw away that first branch.  If it
is, it should optimize it away (and no, these are not real bugs AFAICT).

I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
was fine.  Will try 4.4.0 now.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 04:58:36 pm Rusty Russell wrote:
> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4.1 seems OK (Ubuntu).

This is annoying.  But I'll withdraw the patches; if there's another reason
that 4.4.0 is bad, we can ban it and reintroduce this.  I don't think that
breaking this hack is enough to declare 4.4.0 verboten.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 04:58:36 pm Rusty Russell wrote:
> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4.1 seems OK (Ubuntu).

This is annoying.  But I'll withdraw the patches; if there's another reason
that 4.4.0 is bad, we can ban it and reintroduce this.  I don't think that
breaking this hack is enough to declare 4.4.0 verboten.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 04:58:36 pm Rusty Russell wrote:
> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4.1 seems OK (Ubuntu).

This is annoying.  But I'll withdraw the patches; if there's another reason
that 4.4.0 is bad, we can ban it and reintroduce this.  I don't think that
breaking this hack is enough to declare 4.4.0 verboten.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Huh?  virtio_has_feature does:
> 
> 	if (__builtin_constant_p(fbit))
> 		BUILD_BUG_ON(fbit >= 32);
> 	else
> 		BUG_ON(fbit >= 32);

In Linus' tree (and linux-next) it looks like this:

static inline bool virtio_has_feature(const struct virtio_device *vdev,
				      unsigned int fbit)
{
	/* Did you forget to fix assumptions on max features? */
	MAYBE_BUILD_BUG_ON(fbit >= 32);

	if (fbit < VIRTIO_TRANSPORT_F_START)
		virtio_check_driver_offered_feature(vdev, fbit);

	return test_bit(fbit, vdev->features);
}

> So, if it's not a constant, gcc should throw away that first branch.  If it
> is, it should optimize it away (and no, these are not real bugs AFAICT).

Your version above may well fix the problem.  Alternatively marking it
__always_inline way work as well.

> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4 is more problematic.

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

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Huh?  virtio_has_feature does:
> 
> 	if (__builtin_constant_p(fbit))
> 		BUILD_BUG_ON(fbit >= 32);
> 	else
> 		BUG_ON(fbit >= 32);

In Linus' tree (and linux-next) it looks like this:

static inline bool virtio_has_feature(const struct virtio_device *vdev,
				      unsigned int fbit)
{
	/* Did you forget to fix assumptions on max features? */
	MAYBE_BUILD_BUG_ON(fbit >= 32);

	if (fbit < VIRTIO_TRANSPORT_F_START)
		virtio_check_driver_offered_feature(vdev, fbit);

	return test_bit(fbit, vdev->features);
}

> So, if it's not a constant, gcc should throw away that first branch.  If it
> is, it should optimize it away (and no, these are not real bugs AFAICT).

Your version above may well fix the problem.  Alternatively marking it
__always_inline way work as well.

> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4 is more problematic.

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

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Stephen Rothwell]

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

Hi Rusty,

On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Huh?  virtio_has_feature does:
> 
> 	if (__builtin_constant_p(fbit))
> 		BUILD_BUG_ON(fbit >= 32);
> 	else
> 		BUG_ON(fbit >= 32);

In Linus' tree (and linux-next) it looks like this:

static inline bool virtio_has_feature(const struct virtio_device *vdev,
				      unsigned int fbit)
{
	/* Did you forget to fix assumptions on max features? */
	MAYBE_BUILD_BUG_ON(fbit >= 32);

	if (fbit < VIRTIO_TRANSPORT_F_START)
		virtio_check_driver_offered_feature(vdev, fbit);

	return test_bit(fbit, vdev->features);
}

> So, if it's not a constant, gcc should throw away that first branch.  If it
> is, it should optimize it away (and no, these are not real bugs AFAICT).

Your version above may well fix the problem.  Alternatively marking it
__always_inline way work as well.

> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4 is more problematic.

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

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

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 10:50:20 am Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Tue, 20 Oct 2009 14:15:33 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > +#ifndef __OPTIMIZE__
> > +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > +#else
> > +extern int __build_bug_on_failed;
> > +#define BUILD_BUG_ON(condition)					\
> > +	do {							\
> > +		((void)sizeof(char[1 - 2*!!(condition)]));	\
> > +		if (condition) __build_bug_on_failed = 1;	\
> > +	} while(0)
> > +#endif
> > +#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
> > +
> 
> I decided to try this in linux-next, but an x86_64 allmodconfig build
> gave this (gcc 4.4.0):
> 
> ERROR: "__build_bug_on_failed" [drivers/net/virtio_net.ko] undefined!
> ERROR: "__build_bug_on_failed" [drivers/block/virtio_blk.ko] undefined!
> 
> I assume that this is caused by the "MAYBE_BUILD_BUG_ON(fbit >= 32)" in
> virtio_has_feature() (in include/linux/virtio_config.h) which is called
> all over the place.  Unfortunately, virtio_has_feature() gets uninlined
> in those two files ...

Huh?  virtio_has_feature does:

	if (__builtin_constant_p(fbit))
		BUILD_BUG_ON(fbit >= 32);
	else
		BUG_ON(fbit >= 32);

So, if it's not a constant, gcc should throw away that first branch.  If it
is, it should optimize it away (and no, these are not real bugs AFAICT).

I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
was fine.  Will try 4.4.0 now.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 04:58:36 pm Rusty Russell wrote:
> I did a 4.3.3 allmodconfig with this patch on 64-bit a few days back and all
> was fine.  Will try 4.4.0 now.

4.4.1 seems OK (Ubuntu).

This is annoying.  But I'll withdraw the patches; if there's another reason
that 4.4.0 is bad, we can ban it and reintroduce this.  I don't think that
breaking this hack is enough to declare 4.4.0 verboten.

Thanks,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 05:08:42 pm Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > Huh?  virtio_has_feature does:
> > 
> > 	if (__builtin_constant_p(fbit))
> > 		BUILD_BUG_ON(fbit >= 32);
> > 	else
> > 		BUG_ON(fbit >= 32);
> 
> In Linus' tree (and linux-next) it looks like this:

Ah.  My patch series fixes that as part of removing MAYBE_BUILD_BUG_ON.

I've put both in for linux-next.

Cheers,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 05:08:42 pm Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org> wrote:
> >
> > Huh?  virtio_has_feature does:
> > 
> > 	if (__builtin_constant_p(fbit))
> > 		BUILD_BUG_ON(fbit >= 32);
> > 	else
> > 		BUG_ON(fbit >= 32);
> 
> In Linus' tree (and linux-next) it looks like this:

Ah.  My patch series fixes that as part of removing MAYBE_BUILD_BUG_ON.

I've put both in for linux-next.

Cheers,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 05:08:42 pm Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > Huh?  virtio_has_feature does:
> > 
> > 	if (__builtin_constant_p(fbit))
> > 		BUILD_BUG_ON(fbit >= 32);
> > 	else
> > 		BUG_ON(fbit >= 32);
> 
> In Linus' tree (and linux-next) it looks like this:

Ah.  My patch series fixes that as part of removing MAYBE_BUILD_BUG_ON.

I've put both in for linux-next.

Cheers,
Rusty.

Re: [PATCH] BUILD_BUG_ON: make it handle more cases

[Rusty Russell]

On Thu, 5 Nov 2009 05:08:42 pm Stephen Rothwell wrote:
> Hi Rusty,
> 
> On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > Huh?  virtio_has_feature does:
> > 
> > 	if (__builtin_constant_p(fbit))
> > 		BUILD_BUG_ON(fbit >= 32);
> > 	else
> > 		BUG_ON(fbit >= 32);
> 
> In Linus' tree (and linux-next) it looks like this:

Ah.  My patch series fixes that as part of removing MAYBE_BUILD_BUG_ON.

I've put both in for linux-next.

Cheers,
Rusty.

Re: linux-next: tree build failure

[Liam Girdwood]

Hi Stephen,

On Wed, 2009-12-16 at 18:21 +1100, Stephen Rothwell wrote:

> I guess we knew this was coming :-(

:(

> 
> Caused by a bad rebase of the voltage tree onto Linus' tree (in order to
> do fixups for commit fc7b92fca4e546184557f1c53f84ad57c66b7695 "mfd:
> Rename all twl4030_i2c*" and others).
> 
> I have reverted all the commits in the voltage tree that touched
> drivers/regulator/twl-regulator.c for today:
> 
> 6051d7a2786c57d0c5c4227dd9ddfc348a86ce91 "twl4030-regulator: Fixes VAUX1-3 exclusion introduced"
> 2677c78184a52edcc228c20a1c05c14925118293 "Reset REMAP configuration in regulator probe"
> de449258306274739ed3a747f49b9ac8f91f9e4a "Define critical regulators as always_on"
> 66d65d352f6fd61dd5695925906b140f1e0ac9c3 "twl4030-regulator: Add all TWL regulators to twreg_info"
> f33a0d47bd44dbf44a45c33b32e1e3bbbf8d06e6 "twl4030-regulator: Remove regulator from all groups when disabling"
> 
> Please fix up this tree properly.  You may have been better off merging
> Linus' tree into yours and fixing up the merge commit than trying to do
> the rebase.

Apologies for this. There is a fix coming from Juha today.

Thanks

Liam

Re: linux-next: tree build failure

[Liam Girdwood]

Hi Stephen,

On Wed, 2009-12-16 at 18:21 +1100, Stephen Rothwell wrote:

> I guess we knew this was coming :-(

:(

> 
> Caused by a bad rebase of the voltage tree onto Linus' tree (in order to
> do fixups for commit fc7b92fca4e546184557f1c53f84ad57c66b7695 "mfd:
> Rename all twl4030_i2c*" and others).
> 
> I have reverted all the commits in the voltage tree that touched
> drivers/regulator/twl-regulator.c for today:
> 
> 6051d7a2786c57d0c5c4227dd9ddfc348a86ce91 "twl4030-regulator: Fixes VAUX1-3 exclusion introduced"
> 2677c78184a52edcc228c20a1c05c14925118293 "Reset REMAP configuration in regulator probe"
> de449258306274739ed3a747f49b9ac8f91f9e4a "Define critical regulators as always_on"
> 66d65d352f6fd61dd5695925906b140f1e0ac9c3 "twl4030-regulator: Add all TWL regulators to twreg_info"
> f33a0d47bd44dbf44a45c33b32e1e3bbbf8d06e6 "twl4030-regulator: Remove regulator from all groups when disabling"
> 
> Please fix up this tree properly.  You may have been better off merging
> Linus' tree into yours and fixing up the merge commit than trying to do
> the rebase.

Apologies for this. There is a fix coming from Juha today.

Thanks

Liam

Re: linux-next: tree build failure

[Felipe Balbi]

Hi,

On Wed, Dec 16, 2009 at 08:21:32AM +0100, ext Stephen Rothwell wrote:
>Hi Liam,
>
>Today's linux-next build (powerpc allyesconfig) failed like this:
>
>drivers/regulator/twl-regulator.c: In function 'twlreg_disable':
>drivers/regulator/twl-regulator.c:158: error: 'P1_GRP' undeclared (first use in this function)
>drivers/regulator/twl-regulator.c:158: error: 'P2_GRP' undeclared (first use in this function)
>drivers/regulator/twl-regulator.c:158: error: 'P3_GRP' undeclared (first use in this function)
>drivers/regulator/twl-regulator.c: At top level:
>drivers/regulator/twl-regulator.c:358: error: 'twlldo_list_voltage' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:450:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:450: error: 'TWL_ADJUSTABLE_LDO' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:451:40: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:452:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:453:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:454:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:455:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:456:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:457:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:458:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:459:34: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:460:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:461:40: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
>drivers/regulator/twl-regulator.c:461: error: 'TWL_FIXED_LDO' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:462:39: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:463:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
>drivers/regulator/twl-regulator.c:464:34: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:465:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:466:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
>drivers/regulator/twl-regulator.c:467:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
>drivers/regulator/twl-regulator.c:468:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
>drivers/regulator/twl-regulator.c:469:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
>drivers/regulator/twl-regulator.c:474: error: 'VAUX1_6030_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:474: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:474: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:475: error: 'VAUX2_6030_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:475: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:475: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:476: error: 'VAUX3_6030_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:476: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:476: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:477: error: 'VMMC_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:477: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:477: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:478: error: 'VPP_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:478: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:478: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:479: error: 'VUSIM_VSEL_table' undeclared here (not in a function)
>drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:479: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
>drivers/regulator/twl-regulator.c:479: error: negative width in bit-field '<anonymous>'
>drivers/regulator/twl-regulator.c: In function 'twlreg_probe':
>drivers/regulator/twl-regulator.c:544: error: implicit declaration of function 'twl4030reg_write'
>
>I guess we knew this was coming :-(
>
>Caused by a bad rebase of the voltage tree onto Linus' tree (in order to
>do fixups for commit fc7b92fca4e546184557f1c53f84ad57c66b7695 "mfd:
>Rename all twl4030_i2c*" and others).
>
>I have reverted all the commits in the voltage tree that touched
>drivers/regulator/twl-regulator.c for today:
>
>6051d7a2786c57d0c5c4227dd9ddfc348a86ce91 "twl4030-regulator: Fixes VAUX1-3 exclusion introduced"
>2677c78184a52edcc228c20a1c05c14925118293 "Reset REMAP configuration in regulator probe"
>de449258306274739ed3a747f49b9ac8f91f9e4a "Define critical regulators as always_on"
>66d65d352f6fd61dd5695925906b140f1e0ac9c3 "twl4030-regulator: Add all TWL regulators to twreg_info"
>f33a0d47bd44dbf44a45c33b32e1e3bbbf8d06e6 "twl4030-regulator: Remove regulator from all groups when disabling"
>
>Please fix up this tree properly.  You may have been better off merging
>Linus' tree into yours and fixing up the merge commit than trying to do
>the rebase.

All this crap would not have come if we weren't to rename twl4030 into 
twl. The argument that we need to rename files to make the code reusable 
is plain STUPID.

I made my point several times that we don't need to rename the files to 
make the code reusable, twl4030-*.c were already supporting twl4030, 
twl5030, twl5031 and some of the tps versions. Now several trees are 
broken because of the stupid mess created by one person who can't even 
run make when creating a patch. Now we have to pay the price and have a 
broken bisect point, great.

-- 
balbi

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Liam,

Today's linux-next build (powerpc allyesconfig) failed like this:

drivers/regulator/twl-regulator.c: In function 'twlreg_disable':
drivers/regulator/twl-regulator.c:158: error: 'P1_GRP' undeclared (first use in this function)
drivers/regulator/twl-regulator.c:158: error: 'P2_GRP' undeclared (first use in this function)
drivers/regulator/twl-regulator.c:158: error: 'P3_GRP' undeclared (first use in this function)
drivers/regulator/twl-regulator.c: At top level:
drivers/regulator/twl-regulator.c:358: error: 'twlldo_list_voltage' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:450:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:450: error: 'TWL_ADJUSTABLE_LDO' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:451:40: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:452:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:453:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:454:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:455:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:456:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:457:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:458:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:459:34: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:460:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:461:40: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
drivers/regulator/twl-regulator.c:461: error: 'TWL_FIXED_LDO' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:462:39: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:463:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
drivers/regulator/twl-regulator.c:464:34: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:465:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:466:35: error: macro "TWL_ADJUSTABLE_LDO" requires 4 arguments, but only 3 given
drivers/regulator/twl-regulator.c:467:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
drivers/regulator/twl-regulator.c:468:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
drivers/regulator/twl-regulator.c:469:39: error: macro "TWL_FIXED_LDO" requires 5 arguments, but only 4 given
drivers/regulator/twl-regulator.c:474: error: 'VAUX1_6030_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:474: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:474: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:474: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:475: error: 'VAUX2_6030_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:475: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:475: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:475: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:476: error: 'VAUX3_6030_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:476: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:476: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:476: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:477: error: 'VMMC_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:477: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:477: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:477: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:478: error: 'VPP_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:478: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:478: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:478: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:479: error: 'VUSIM_VSEL_table' undeclared here (not in a function)
drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:479: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:479: warning: type defaults to 'int' in declaration of 'type name'
drivers/regulator/twl-regulator.c:479: error: negative width in bit-field '<anonymous>'
drivers/regulator/twl-regulator.c: In function 'twlreg_probe':
drivers/regulator/twl-regulator.c:544: error: implicit declaration of function 'twl4030reg_write'

I guess we knew this was coming :-(

Caused by a bad rebase of the voltage tree onto Linus' tree (in order to
do fixups for commit fc7b92fca4e546184557f1c53f84ad57c66b7695 "mfd:
Rename all twl4030_i2c*" and others).

I have reverted all the commits in the voltage tree that touched
drivers/regulator/twl-regulator.c for today:

6051d7a2786c57d0c5c4227dd9ddfc348a86ce91 "twl4030-regulator: Fixes VAUX1-3 exclusion introduced"
2677c78184a52edcc228c20a1c05c14925118293 "Reset REMAP configuration in regulator probe"
de449258306274739ed3a747f49b9ac8f91f9e4a "Define critical regulators as always_on"
66d65d352f6fd61dd5695925906b140f1e0ac9c3 "twl4030-regulator: Add all TWL regulators to twreg_info"
f33a0d47bd44dbf44a45c33b32e1e3bbbf8d06e6 "twl4030-regulator: Remove regulator from all groups when disabling"

Please fix up this tree properly.  You may have been better off merging
Linus' tree into yours and fixing up the merge commit than trying to do
the rebase.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: linux-next: tree build failure

[Stephen Rothwell]

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

Hi Jens,

On Thu, 1 Oct 2009 09:58:38 +0200 Jens Axboe <jens.axboe@oracle.com> wrote:
>
> Sorry Stephen, fixed up.

Thanks.

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

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

Re: linux-next: tree build failure

[Jens Axboe]

On Thu, Oct 01 2009, Stephen Rothwell wrote:
> Hi Jens,
> 
> Today's linux-next build (powerpc allnoconfig) failed like this:
> 
> include/linux/blktrace_api.h:215:40: error: macro parameters must be comma-separated
> 
> Caused by commit 1e03edf3b5b16f7f30fa1e397cc9a130305fa8d3 ("Add missing
> blk_trace_remove_sysfs to be in pair with blk_trace_init_sysfs") from the
> block tree.  Half way between a static inline and a macro ...

Sorry Stephen, fixed up.

-- 
Jens Axboe

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Jens,

Today's linux-next build (powerpc allnoconfig) failed like this:

include/linux/blktrace_api.h:215:40: error: macro parameters must be comma-separated

Caused by commit 1e03edf3b5b16f7f30fa1e397cc9a130305fa8d3 ("Add missing
blk_trace_remove_sysfs to be in pair with blk_trace_init_sysfs") from the
block tree.  Half way between a static inline and a macro ...

I have reverted that commit for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

linux-next: tree build failure

[Stephen Rothwell]

Hi Rafael,

Today's linux-next build (powerpc allyesconfig) failed like this:

drivers/base/power/runtime.c:948: error: __ksymtab_pm_runtime_disable causes a section type conflict

Caused by commit d9d4cc5169ca18df9ff5afd31c6e6b715ecb454a ("PM: Introduce
core framework for run-time PM of I/O devices (rev. 17)") from the
suspend tree.  This commit EXPORTs pm_runtime_disable which is an inline
function that calls __pm_runtime_disable (which is probably what was
meant to be EXPORTed).

I wish we could get these type of errors to fail on x86 as well ...

I have applied the following patch for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 17 Aug 2009 18:34:28 +1000
Subject: [PATCH] suspend: EXPORT the correct function.

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

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 28a3f91..38556f6 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -945,7 +945,7 @@ void __pm_runtime_disable(struct device *dev, bool check_resume)
  out:
 	spin_unlock_irq(&dev->power.lock);
 }
-EXPORT_SYMBOL_GPL(pm_runtime_disable);
+EXPORT_SYMBOL_GPL(__pm_runtime_disable);
 
 /**
  * pm_runtime_enable - Enable run-time PM of a device.
-- 
1.6.3.3

Re: linux-next: tree build failure

[Stephen Rothwell]

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

Hi Neil,

On Mon, 3 Aug 2009 11:01:24 +1000 (EST) "NeilBrown" <neilb@suse.de> wrote:
>
> On Mon, August 3, 2009 10:35 am, Stephen Rothwell wrote:
> >
> > Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> >
> > ERROR: ".md_integrity_add_rdev" [drivers/md/multipath.ko] undefined!
> >
> > Caused by commit 66f4f3f41f5c334f399e920b2aaad9b82514acda ("md: Push down
> > data integrity code to personalities") from the md-current and md trees.
> > Looks like md_integrity_add_rdev needs an EXPORT_SYMBOL.
> 
> Yes, of course...
> Thanks!
> Fixed.

OK, so I refetched the md tree and will use it 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: tree build failure

[NeilBrown]

On Mon, August 3, 2009 10:35 am, Stephen Rothwell wrote:
> Hi Neil,
>
> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
>
> ERROR: ".md_integrity_add_rdev" [drivers/md/multipath.ko] undefined!
>
> Caused by commit 66f4f3f41f5c334f399e920b2aaad9b82514acda ("md: Push down
> data integrity code to personalities") from the md-current and md trees.
> Looks like md_integrity_add_rdev needs an EXPORT_SYMBOL.

Yes, of course...
Thanks!
Fixed.

NeilBrown


>
> I have used the version of the md-current and md trees from next-20090731
> for today.
> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/
>

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Neil,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

ERROR: ".md_integrity_add_rdev" [drivers/md/multipath.ko] undefined!

Caused by commit 66f4f3f41f5c334f399e920b2aaad9b82514acda ("md: Push down
data integrity code to personalities") from the md-current and md trees.
Looks like md_integrity_add_rdev needs an EXPORT_SYMBOL.

I have used the version of the md-current and md trees from next-20090731
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: tree build failure

[Stephen Rothwell]

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

Hi Dave, Karsten,

On Mon, 27 Jul 2009 08:06:17 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
> From: Karsten Keil <keil@b1-systems.de>
> Date: Mon, 27 Jul 2009 11:21:33 +0200
> 
> > Sorry for that, yes this fix is correct - I really should test allyesconfig 
> > more times by myself  :-(.

I don't really expect people to do allyesconfig build very often.

> > ACK.
> 
> Applied.

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: tree build failure

[David Miller]

From: Karsten Keil <keil@b1-systems.de>
Date: Mon, 27 Jul 2009 11:21:33 +0200

> On Montag, 27. Juli 2009 09:53:28 you wrote:
>> Hi Dave,
>>
>> Today's linux-next build (powerpc allyesconfig) failed like this:
>>
>> drivers/isdn/hisax/built-in.o: In function `setup_w6692':
>> (.opd+0x4a28): multiple definition of `setup_w6692'
>> drivers/isdn/hardware/built-in.o:(.opd+0x4e90): first defined here
>> drivers/isdn/hisax/built-in.o: In function `.setup_w6692':
>> (.devinit.text+0x5b14): multiple definition of `.setup_w6692'
>> drivers/isdn/hardware/built-in.o:(.text+0x8b75c): first defined here
>>
>> Caused by commit 707b2ce6c1f4f1261788f2ff09ad82c35e0e6240
>> ("mISDN: Add driver for Winbond cards").
>>
>> setup_w6692 appears in drivers/isdn/hardware/mISDN/w6692.c and
>> drivers/isdn/hisax/w6692.c.  I applied the patch below for today (this
>> may not be correct).
>>
> 
> Sorry for that, yes this fix is correct - I really should test allyesconfig 
> more times by myself  :-(.
> 
> ACK.

Applied.

Re: linux-next: tree build failure

[Karsten Keil]

Hi Stephen,


On Montag, 27. Juli 2009 09:53:28 you wrote:
> Hi Dave,
>
> Today's linux-next build (powerpc allyesconfig) failed like this:
>
> drivers/isdn/hisax/built-in.o: In function `setup_w6692':
> (.opd+0x4a28): multiple definition of `setup_w6692'
> drivers/isdn/hardware/built-in.o:(.opd+0x4e90): first defined here
> drivers/isdn/hisax/built-in.o: In function `.setup_w6692':
> (.devinit.text+0x5b14): multiple definition of `.setup_w6692'
> drivers/isdn/hardware/built-in.o:(.text+0x8b75c): first defined here
>
> Caused by commit 707b2ce6c1f4f1261788f2ff09ad82c35e0e6240
> ("mISDN: Add driver for Winbond cards").
>
> setup_w6692 appears in drivers/isdn/hardware/mISDN/w6692.c and
> drivers/isdn/hisax/w6692.c.  I applied the patch below for today (this
> may not be correct).
>

Sorry for that, yes this fix is correct - I really should test allyesconfig 
more times by myself  :-(.

ACK.

> [I also get these warnings:
> drivers/isdn/hardware/mISDN/w6692.c:533: warning: 'setvolume' defined but
> not used drivers/isdn/hardware/mISDN/w6692.c:560: warning: 'enable_pots'
> defined but not used ]

The POTS code is implemented, but the higher level interface is not finally 
defined yet, so the two functions are not used at the moment.
The interface will added soon.


> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Mon, 27 Jul 2009 17:45:36 +1000
> Subject: [PATCH] net: fix multiple definitions of setup_w6692
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/isdn/hardware/mISDN/w6692.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/isdn/hardware/mISDN/w6692.c
> b/drivers/isdn/hardware/mISDN/w6692.c index 1b9008f..d3f1077 100644
> --- a/drivers/isdn/hardware/mISDN/w6692.c
> +++ b/drivers/isdn/hardware/mISDN/w6692.c
> @@ -1234,7 +1234,7 @@ w6692_dctrl(struct mISDNchannel *ch, u32 cmd, void
> *arg) return err;
>  }
>
> -int
> +static int
>  setup_w6692(struct w6692_hw *card)
>  {
>  	u32	val;

linux-next: tree build failure

[Stephen Rothwell]

Hi Dave,

Today's linux-next build (powerpc allyesconfig) failed like this:

drivers/isdn/hisax/built-in.o: In function `setup_w6692':
(.opd+0x4a28): multiple definition of `setup_w6692'
drivers/isdn/hardware/built-in.o:(.opd+0x4e90): first defined here
drivers/isdn/hisax/built-in.o: In function `.setup_w6692':
(.devinit.text+0x5b14): multiple definition of `.setup_w6692'
drivers/isdn/hardware/built-in.o:(.text+0x8b75c): first defined here

Caused by commit 707b2ce6c1f4f1261788f2ff09ad82c35e0e6240
("mISDN: Add driver for Winbond cards").

setup_w6692 appears in drivers/isdn/hardware/mISDN/w6692.c and
drivers/isdn/hisax/w6692.c.  I applied the patch below for today (this
may not be correct).

[I also get these warnings:
drivers/isdn/hardware/mISDN/w6692.c:533: warning: 'setvolume' defined but not used
drivers/isdn/hardware/mISDN/w6692.c:560: warning: 'enable_pots' defined but not used
]
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 27 Jul 2009 17:45:36 +1000
Subject: [PATCH] net: fix multiple definitions of setup_w6692

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

diff --git a/drivers/isdn/hardware/mISDN/w6692.c b/drivers/isdn/hardware/mISDN/w6692.c
index 1b9008f..d3f1077 100644
--- a/drivers/isdn/hardware/mISDN/w6692.c
+++ b/drivers/isdn/hardware/mISDN/w6692.c
@@ -1234,7 +1234,7 @@ w6692_dctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
 	return err;
 }
 
-int
+static int
 setup_w6692(struct w6692_hw *card)
 {
 	u32	val;
-- 
1.6.3.3

Re: linux-next: tree build failure

[Takashi Iwai]

At Tue, 7 Apr 2009 13:41:35 +1000,
Stephen Rothwell wrote:
> 
> Hi Takashi,
> 
> Today's linux-next build (powerpc allyesconfig) failed like this:
> 
> sound/soc/codecs/twl4030.c: In function 'twl4030_startup':
> sound/soc/codecs/twl4030.c:1227: error: 'struct snd_soc_device' has no member named 'codec'
> sound/soc/codecs/twl4030.c: In function 'twl4030_shutdown':
> sound/soc/codecs/twl4030.c:1258: error: 'struct snd_soc_device' has no member named 'codec'
> sound/soc/codecs/twl4030.c: At top level:
> sound/soc/codecs/twl4030.c:1442: warning: initialization from incompatible pointer type
> sound/soc/codecs/twl4030.c:1443: warning: initialization from incompatible pointer type
> 
> Caused by commit 7220b9f4bd4fad41f6f7299fe74c2c38ec85d793 ("ASoC:
> TWL4030: Add constrains for second stream") from the sound-current tree.
> 
> Was this even build tested?
> 
> I have reverted that commit for today.

Sorry for that.  I'm still on vacation now and have only very limited
remote access, thus couldn't do full cross-platform build tests...

Anyway, Mark already submitted the fix and Linus merged it in, so
everything should be OK now.


thanks,

Takashi

Re: linux-next: tree build failure

[Mark Brown]

The following changes since commit 5c15a6869a75000fecea61e9985f4753311ec534:
  Anton Vorontsov (1):
        ASoC: fsl_dma: Pass the proper device for dma mapping routines

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git for-2.6.30

Peter Ujfalusi (1):
      ASoC: TWL4030: Compillation error fix

 sound/soc/codecs/twl4030.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Takashi,

Today's linux-next build (powerpc allyesconfig) failed like this:

sound/soc/codecs/twl4030.c: In function 'twl4030_startup':
sound/soc/codecs/twl4030.c:1227: error: 'struct snd_soc_device' has no member named 'codec'
sound/soc/codecs/twl4030.c: In function 'twl4030_shutdown':
sound/soc/codecs/twl4030.c:1258: error: 'struct snd_soc_device' has no member named 'codec'
sound/soc/codecs/twl4030.c: At top level:
sound/soc/codecs/twl4030.c:1442: warning: initialization from incompatible pointer type
sound/soc/codecs/twl4030.c:1443: warning: initialization from incompatible pointer type

Caused by commit 7220b9f4bd4fad41f6f7299fe74c2c38ec85d793 ("ASoC:
TWL4030: Add constrains for second stream") from the sound-current tree.

Was this even build tested?

I have reverted that commit 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: tree build failure

[Stephen Rothwell]

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

Hi Mauro,

On Mon, 23 Mar 2009 19:27:09 -0300 Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
>
> Thanks. I just fold the fix at c69a6f8d071763 to avoid bisect troubles.

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 --]

Re: linux-next: tree build failure

[Mauro Carvalho Chehab]

On Mon, 23 Mar 2009 20:38:10 +1100
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Mauro,
> 
> Today's linux-next build (powerpc allyesconfig) failed like this:
> 
> drivers/media/video/ov7670.c: In function 'ov7670_write':
> drivers/media/video/ov7670.c:433: error: implicit declaration of function 'msleep'
> 
> Caused by commit c69a6f8d071763141e774893c71e09ded722f05d ("V4L/DVB
> (11113): ov7670: convert to v4l2_subdev") which removed the include of
> linux/delay.h ...
> 
> I have reverted that commit (and the two dependent commits
> b7db06380e43161560a0eb2cc5fe891c23994b35 ("V4L/DVB (11116): ov7670:
> cleanup and remove legacy code") and
> 2134b61009c102142e5140ab156d1cd99e0f6362 ("V4L/DVB (11117): ov7670: add
> support to get/set registers")) for today.
> 
Thanks. I just fold the fix at c69a6f8d071763 to avoid bisect troubles.

Cheers,
Mauro

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Mauro,

Today's linux-next build (powerpc allyesconfig) failed like this:

drivers/media/video/ov7670.c: In function 'ov7670_write':
drivers/media/video/ov7670.c:433: error: implicit declaration of function 'msleep'

Caused by commit c69a6f8d071763141e774893c71e09ded722f05d ("V4L/DVB
(11113): ov7670: convert to v4l2_subdev") which removed the include of
linux/delay.h ...

I have reverted that commit (and the two dependent commits
b7db06380e43161560a0eb2cc5fe891c23994b35 ("V4L/DVB (11116): ov7670:
cleanup and remove legacy code") and
2134b61009c102142e5140ab156d1cd99e0f6362 ("V4L/DVB (11117): ov7670: add
support to get/set registers")) 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: tree build failure

[Rusty Russell]

On Thursday 05 March 2009 18:11:07 Stephen Rothwell wrote:
> Caused by commit 6034a37f9dff70fea021bf3a2dcdc356bccb88b3
> ("cpumask:use-cpu_sibling_mask") which introduced uses of
> cpu_sibling_mask which is only defined for x86 ...

Thanks.  I've moved that patch back.

I *think* it can use topology_thread_cpumask(), but I have to check that it's defined in all the right cases.

Thanks very much,
Rusty.

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Rusty,

Today's linux-next build (powerpc allyesconfig) failed like this:

kernel/sched.c: In function 'cpu_to_phys_group':
kernel/sched.c:7437: error: implicit declaration of function 'cpu_sibling_mask'
kernel/sched.c:7437: warning: passing argument 2 of 'cpumask_and' makes pointer from integer without a cast
kernel/sched.c: In function '__build_sched_domains':
kernel/sched.c:7780: warning: passing argument 2 of 'cpumask_and' makes pointer from integer without a cast
kernel/sched.c:7790: warning: passing argument 2 of 'cpumask_and' makes pointer from integer without a cast

Caused by commit 6034a37f9dff70fea021bf3a2dcdc356bccb88b3
("cpumask:use-cpu_sibling_mask") which introduced uses of
cpu_sibling_mask which is only defined for x86 ...

I have reverted that commit 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: tree build failure

[Ingo Molnar]

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

> Hi Ingo,
> 
> On Fri, 16 Jan 2009 11:53:24 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > 
> > > Today's linux-next build (powerpc allnoconfig) failed like this:
> > > 
> > > In file included from kernel/sched.c:1703:
> > > kernel/sched_fair.c: In function 'adaptive_gran':
> > > kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > > 
> > > Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
> > > ("sched: prefer wakers") from the sched tree.
> > 
> > that commit builds just fine on x86 and on powerpc as well.
> 
> The build was an allnoconfig build, so CONFIG_SCHEDSTATS was not set.

ah, i see. So there's nothing powerpc-specific about this, it fails on x86 
allnoconfig just as much? Is this because you test powerpc as the first 
architecture in the linux-next builds?

I think i see where the mismerge comes from: you dont have the 
perfcounters tree in linux-next which masked this interim stage. I've 
fixed this via the commit below.

	Ingo

-------------------->
>From 34cb61359b503d7aff6447acb037a5efd6ce93b2 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 16 Jan 2009 13:36:06 +0100
Subject: [PATCH] sched: fix !CONFIG_SCHEDSTATS build failure

Stephen Rothwell reported this linux-next build failure with !CONFIG_SCHEDSTATS:

| In file included from kernel/sched.c:1703:
| kernel/sched_fair.c: In function 'adaptive_gran':
| kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'

The start_runtime and avg_wakeup metrics are now not just for statistics,
but also for scheduling - so they always need to be available. (Also
move out the nr_migrations fields - for future perfcounters usage.)

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/sched.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index daf4e07..5d56b54 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1031,6 +1031,10 @@ struct sched_entity {
 	u64			last_wakeup;
 	u64			avg_overlap;
 
+	u64			start_runtime;
+	u64			avg_wakeup;
+	u64			nr_migrations;
+
 #ifdef CONFIG_SCHEDSTATS
 	u64			wait_start;
 	u64			wait_max;
@@ -1046,10 +1050,6 @@ struct sched_entity {
 	u64			exec_max;
 	u64			slice_max;
 
-	u64			start_runtime;
-	u64			avg_wakeup;
-
-	u64			nr_migrations;
 	u64			nr_migrations_cold;
 	u64			nr_failed_migrations_affine;
 	u64			nr_failed_migrations_running;

Re: linux-next: tree build failure

[Stephen Rothwell]

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

Hi Ingo,

On Fri, 16 Jan 2009 11:53:24 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Today's linux-next build (powerpc allnoconfig) failed like this:
> > 
> > In file included from kernel/sched.c:1703:
> > kernel/sched_fair.c: In function 'adaptive_gran':
> > kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > 
> > Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
> > ("sched: prefer wakers") from the sched tree.
> 
> that commit builds just fine on x86 and on powerpc as well.

The build was an allnoconfig build, so CONFIG_SCHEDSTATS was not set.

> Also, at commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565 there's no 
> 'sched_entity' use on line 1324 of kernel/sched_fair.c. Could you tell me 
> the exact commit ID you got the build failure under?

Commit b88402a67e74222061d2f24aef90ee58d2a3f080 "Merge commit
'kmemleak/kmemleak'" (what you get if you remove the last two commits
form today's linux-next).

At that point, kernel/sched_fair.c contains (from line 1320):

static unsigned long
adaptive_gran(struct sched_entity *curr, struct sched_entity *se)
{
	u64 this_run = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
	u64 expected_wakeup = 2*se->avg_wakeup * cfs_rq_of(se)->nr_running;
	u64 gran = 0;

	if (this_run < expected_wakeup)
		gran = expected_wakeup - this_run;

	return min_t(s64, gran, sysctl_sched_wakeup_granularity);
}

That function was added by commit
e52fb7c097238d34f4d8e2a596f8a3f85b0c0565.  Also, at that commit, stuct
sched_entity (in sched.h) looks like this:

struct sched_entity {
	.
	.

	u64			last_wakeup;
	u64			avg_overlap;

#ifdef CONFIG_SCHEDSTATS
	u64			wait_start;
	u64			wait_max;
	.
	.

	u64			start_runtime;
	u64			avg_wakeup;

	.
	.
	u64			nr_wakeups_passive;
	u64			nr_wakeups_idle;
#endif

	.
	.
};

The two fields "start_runtime" and "avg_wakeup" were added by the
previous commit 831451ac4e44d3a20b581ce726ef1d1144373f7d "sched:
introduce avg_wakeup" but according to Peter's email, they were added in
the wrong place.
-- 
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: tree build failure

[Ingo Molnar]

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

> Hi ,
> 
> Today's linux-next build (powerpc allnoconfig) failed like this:
> 
> In file included from kernel/sched.c:1703:
> kernel/sched_fair.c: In function 'adaptive_gran':
> kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'
> 
> Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
> ("sched: prefer wakers") from the sched tree.

that commit builds just fine on x86 and on powerpc as well.

Also, at commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565 there's no 
'sched_entity' use on line 1324 of kernel/sched_fair.c. Could you tell me 
the exact commit ID you got the build failure under?

	Ingo

Re: linux-next: tree build failure

[Stephen Rothwell]

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

On Fri, 16 Jan 2009 10:03:55 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> > That smells like a bad merge somewhere, I just checked -tip and it looks 
> > like so:
> 
> yep. I zapped the auto-sched-next branch for now.

Glad to be of service.

-- 
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: tree build failure

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Fri, 2009-01-16 at 16:37 +1100, Stephen Rothwell wrote:
> > Hi ,
> > 
> > Today's linux-next build (powerpc allnoconfig) failed like this:
> > 
> > In file included from kernel/sched.c:1703:
> > kernel/sched_fair.c: In function 'adaptive_gran':
> > kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > 
> > Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
> > ("sched: prefer wakers") from the sched tree.
> > 
> > kernel/sched.c: In function 'enqueue_task':
> > kernel/sched.c:1752: error: 'struct sched_entity' has no member named 'start_runtime'
> > kernel/sched.c: In function 'dequeue_task':
> > kernel/sched.c:1767: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > kernel/sched.c: In function 'try_to_wake_up':
> > kernel/sched.c:2409: error: 'struct sched_entity' has no member named 'start_runtime'
> > kernel/sched.c:2410: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > kernel/sched.c: In function '__sched_fork':
> > kernel/sched.c:2454: error: 'struct sched_entity' has no member named 'start_runtime'
> > kernel/sched.c:2455: error: 'struct sched_entity' has no member named 'avg_wakeup'
> > 
> > Caused by commit 831451ac4e44d3a20b581ce726ef1d1144373f7d ("sched:
> > introduce avg_wakeup") from the sched tree.
> > 
> > These members depend on CONFIG_SCHEDSTATS.
> 
> That smells like a bad merge somewhere, I just checked -tip and it looks 
> like so:

yep. I zapped the auto-sched-next branch for now.

	Ingo

Re: linux-next: tree build failure

[Peter Zijlstra]

On Fri, 2009-01-16 at 16:37 +1100, Stephen Rothwell wrote:
> Hi ,
> 
> Today's linux-next build (powerpc allnoconfig) failed like this:
> 
> In file included from kernel/sched.c:1703:
> kernel/sched_fair.c: In function 'adaptive_gran':
> kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'
> 
> Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
> ("sched: prefer wakers") from the sched tree.
> 
> kernel/sched.c: In function 'enqueue_task':
> kernel/sched.c:1752: error: 'struct sched_entity' has no member named 'start_runtime'
> kernel/sched.c: In function 'dequeue_task':
> kernel/sched.c:1767: error: 'struct sched_entity' has no member named 'avg_wakeup'
> kernel/sched.c: In function 'try_to_wake_up':
> kernel/sched.c:2409: error: 'struct sched_entity' has no member named 'start_runtime'
> kernel/sched.c:2410: error: 'struct sched_entity' has no member named 'avg_wakeup'
> kernel/sched.c: In function '__sched_fork':
> kernel/sched.c:2454: error: 'struct sched_entity' has no member named 'start_runtime'
> kernel/sched.c:2455: error: 'struct sched_entity' has no member named 'avg_wakeup'
> 
> Caused by commit 831451ac4e44d3a20b581ce726ef1d1144373f7d ("sched:
> introduce avg_wakeup") from the sched tree.
> 
> These members depend on CONFIG_SCHEDSTATS.

That smells like a bad merge somewhere, I just checked -tip and it looks
like so:

struct sched_entity {
	...
        u64                     avg_overlap;

        u64                     start_runtime;
        u64                     avg_wakeup;

        u64                     nr_migrations;

#ifdef CONFIG_SCHEDSTATS
        u64                     wait_start;
        u64                     wait_max;
	...
};

linux-next: tree build failure

[Stephen Rothwell]

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

Hi ,

Today's linux-next build (powerpc allnoconfig) failed like this:

In file included from kernel/sched.c:1703:
kernel/sched_fair.c: In function 'adaptive_gran':
kernel/sched_fair.c:1324: error: 'struct sched_entity' has no member named 'avg_wakeup'

Caused by commit e52fb7c097238d34f4d8e2a596f8a3f85b0c0565
("sched: prefer wakers") from the sched tree.

kernel/sched.c: In function 'enqueue_task':
kernel/sched.c:1752: error: 'struct sched_entity' has no member named 'start_runtime'
kernel/sched.c: In function 'dequeue_task':
kernel/sched.c:1767: error: 'struct sched_entity' has no member named 'avg_wakeup'
kernel/sched.c: In function 'try_to_wake_up':
kernel/sched.c:2409: error: 'struct sched_entity' has no member named 'start_runtime'
kernel/sched.c:2410: error: 'struct sched_entity' has no member named 'avg_wakeup'
kernel/sched.c: In function '__sched_fork':
kernel/sched.c:2454: error: 'struct sched_entity' has no member named 'start_runtime'
kernel/sched.c:2455: error: 'struct sched_entity' has no member named 'avg_wakeup'

Caused by commit 831451ac4e44d3a20b581ce726ef1d1144373f7d ("sched:
introduce avg_wakeup") from the sched tree.

These members depend on CONFIG_SCHEDSTATS.

I have reverted the sched 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: tree build failure

[Takashi Iwai]

At Fri, 28 Nov 2008 21:43:15 +1100,
Stephen Rothwell wrote:
> 
> [1  <text/plain; US-ASCII (quoted-printable)>]
> Hi Takashi,
> 
> On Fri, 28 Nov 2008 11:25:22 +0100 Takashi Iwai <tiwai@suse.de> wrote:
> >
> > It was already fixed on sound.git tree in this morning.
> > Pull again if possible.
> 
> On Monday ...
> 
> Thanks for fixing it.  Please be a bit more careful about what gets put
> into your for-next branch.

Yep, sorry.
I tested allyesconfig and randconfig, but forgot defconfig through my
build tests.


Takashi

Re: linux-next: tree build failure

[Stephen Rothwell]

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

Hi Takashi,

On Fri, 28 Nov 2008 11:25:22 +0100 Takashi Iwai <tiwai@suse.de> wrote:
>
> It was already fixed on sound.git tree in this morning.
> Pull again if possible.

On Monday ...

Thanks for fixing it.  Please be a bit more careful about what gets put
into your for-next branch.

-- 
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: tree build failure

[Takashi Iwai]

At Fri, 28 Nov 2008 21:04:12 +1100,
Stephen Rothwell wrote:
> 
> Hi Takashi,
> 
> Today's linux-next build (i386 defconfig) failed like this:
> 
> sound/pci/hda/hda_intel.c: In function 'azx_codec_create':
> sound/pci/hda/hda_intel.c:1236: error: 'power_save' undeclared (first use in this function)
> 
> Caused by commit fee2fba3586f78762ecc5f432dfd3602765a31b3 ("ALSA: hda -
> Move power_save option to hda_intel.c").
> 
> I have reverted that commit.  It needs to be fixed soon.

It was already fixed on sound.git tree in this morning.
Pull again if possible.


thanks,

Takashi

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Takashi,

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

sound/pci/hda/hda_intel.c: In function 'azx_codec_create':
sound/pci/hda/hda_intel.c:1236: error: 'power_save' undeclared (first use in this function)

Caused by commit fee2fba3586f78762ecc5f432dfd3602765a31b3 ("ALSA: hda -
Move power_save option to hda_intel.c").

I have reverted that commit.  It needs to be fixed soon.
-- 
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: tree build failure

[Stephen Rothwell]

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

Hi Linus,

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

drivers/gpu/drm/i915/i915_gem.c: In function 'fast_user_write':
drivers/gpu/drm/i915/i915_gem.c:196: error: 'o' undeclared (first use in this function)

Caused by commit 9b7530cc329eb036cfa589930c270e85031f554c ("i915: cleanup
coding horrors in i915_gem_gtt_pwrite()").

I have applied Thomas' fix since today' linux-next base precedes the fix
being added.
-- 
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: tree build failure

[Steven Rostedt]

On Mon, 25 Aug 2008, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc allyesconfig) failed like this:
> 
> kernel/trace/ftrace.c:317: error: redefinition of 'ftrace_release'
> include/linux/ftrace.h:207: error: previous definition of 'ftrace_release' was here
> 
> Introduced by commit 61e679bf257baf9a089641999cf618e92f07b5dd ("ftrace:
> remove old pointers to mcount").
> 
> I applied the following patch.

I'm also working on PPC right now, and saw this. I wrote a patch up, since 
it actually can be used with either version of the Dynamic Tracing.
We can keep this one for now, util I solve some of the issues I'm working 
on.

Thanks,

-- Steve

linux-next: tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (powerpc allyesconfig) failed like this:

kernel/trace/ftrace.c:317: error: redefinition of 'ftrace_release'
include/linux/ftrace.h:207: error: previous definition of 'ftrace_release' was here

Introduced by commit 61e679bf257baf9a089641999cf618e92f07b5dd ("ftrace:
remove old pointers to mcount").

I 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 20:28:31 +1000
Subject: [PATCH] ftrace: protect the definition of ftrace_release

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

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 2bc3eb5..094170a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -314,6 +314,7 @@ static void ftrace_free_rec(struct dyn_ftrace *rec)
 	rec->flags |= FTRACE_FL_FREE;
 }
 
+#ifdef CONFIG_FTRACE_MCOUNT_RECORD
 void ftrace_release(void *start, unsigned long size)
 {
 	struct dyn_ftrace *rec;
@@ -339,6 +340,7 @@ void ftrace_release(void *start, unsigned long size)
 	spin_unlock(&ftrace_lock);
 
 }
+#endif /* CONFIG_FTRACE_MCOUNT_RECORD */
 
 static struct dyn_ftrace *ftrace_alloc_dyn_node(unsigned long ip)
 {
-- 
1.5.6.3

linux-next: tree build failure

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next build (powerpc g5_defconfig) failed like this:

drivers/usb/class/cdc-acm.c: In function 'acm_suspend':
drivers/usb/class/cdc-acm.c:1263: error: 'struct usb_device' has no member named 'auto_pm'

Caused by commit 6a5aa976ea24ea5fe616e278e80d8986a159c1c1 ("USB:
additional power savings for cdc-acm devices that support remote wakeup")
(from the usb tree) and the lack of CCONFIG_PM ...

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

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