linux-next build failure

linux-next build failure

[Mark Salter]

This patch causes a build failure for no-MMU builds:

  commit 41df2957cb010edfa2f9d394d3617da6beeeb660
  Author: Colin Cross <ccross@android.com>
  Date:   Wed Oct 30 11:45:39 2013 +1100

      mm: add a field to store names for private anonymous memory
    
The problem is:

  kernel/built-in.o: In function `sys_prctl':
  (.text+0x19854): undefined reference to `madvise_set_anon_name'

madvise doesn't make sense without an MMU.

--Mark

Re: linux-next build failure

[Stephen Rothwell]

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

[Jut add Andrew to the cc list]

On Wed, 30 Oct 2013 11:27:45 -0400 Mark Salter <msalter@redhat.com> wrote:
>
> This patch causes a build failure for no-MMU builds:
> 
>   commit 41df2957cb010edfa2f9d394d3617da6beeeb660
>   Author: Colin Cross <ccross@android.com>
>   Date:   Wed Oct 30 11:45:39 2013 +1100
> 
>       mm: add a field to store names for private anonymous memory
>     
> The problem is:
> 
>   kernel/built-in.o: In function `sys_prctl':
>   (.text+0x19854): undefined reference to `madvise_set_anon_name'
> 
> madvise doesn't make sense without an MMU.
> 
> --Mark

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

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

Re: linux-next build failure

[Andrew Morton]

On Thu, 31 Oct 2013 07:37:00 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> [Jut add Andrew to the cc list]
> 
> On Wed, 30 Oct 2013 11:27:45 -0400 Mark Salter <msalter@redhat.com> wrote:
> >
> > This patch causes a build failure for no-MMU builds:
> > 
> >   commit 41df2957cb010edfa2f9d394d3617da6beeeb660
> >   Author: Colin Cross <ccross@android.com>
> >   Date:   Wed Oct 30 11:45:39 2013 +1100
> > 
> >       mm: add a field to store names for private anonymous memory
> >     
> > The problem is:
> > 
> >   kernel/built-in.o: In function `sys_prctl':
> >   (.text+0x19854): undefined reference to `madvise_set_anon_name'
> > 
> > madvise doesn't make sense without an MMU.
> > 
> > --Mark

OK, thanks.  I'll drop

mm-rearrange-madvise-code-to-allow-for-reuse.patch
mm-add-a-field-to-store-names-for-private-anonymous-memory.patch
mm-add-a-field-to-store-names-for-private-anonymous-memory-fix.patch
mm-add-a-field-to-store-names-for-private-anonymous-memory-fix-fix-2.patch

for now.  They have other question marks - let's revisit everything
after 3.13-rc1.

Re: linux-next build failure

[Stephen Rothwell]

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

Hi Andrew,

On Wed, 30 Oct 2013 13:50:42 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 31 Oct 2013 07:37:00 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > [Jut add Andrew to the cc list]
> > 
> > On Wed, 30 Oct 2013 11:27:45 -0400 Mark Salter <msalter@redhat.com> wrote:
> > >
> > > This patch causes a build failure for no-MMU builds:
> > > 
> > >   commit 41df2957cb010edfa2f9d394d3617da6beeeb660
> > >   Author: Colin Cross <ccross@android.com>
> > >   Date:   Wed Oct 30 11:45:39 2013 +1100
> > > 
> > >       mm: add a field to store names for private anonymous memory
> > >     
> > > The problem is:
> > > 
> > >   kernel/built-in.o: In function `sys_prctl':
> > >   (.text+0x19854): undefined reference to `madvise_set_anon_name'
> > > 
> > > madvise doesn't make sense without an MMU.
> > > 
> > > --Mark
> 
> OK, thanks.  I'll drop
> 
> mm-rearrange-madvise-code-to-allow-for-reuse.patch
> mm-add-a-field-to-store-names-for-private-anonymous-memory.patch
> mm-add-a-field-to-store-names-for-private-anonymous-memory-fix.patch
> mm-add-a-field-to-store-names-for-private-anonymous-memory-fix-fix-2.patch
> 
> for now.  They have other question marks - let's revisit everything
> after 3.13-rc1.

I have removed them from my copy of the mmotm tree ...

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

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

linux-next build failure

[Mark Salter]

I'm seeing a build failure for a no-MMU system:

   mm/built-in.o: In function `__vm_enough_memory':
  (.text+0x1ce70): undefined reference to `vm_commit_limit'
  fs/built-in.o: In function `meminfo_proc_show':
  meminfo.c:(.text+0x6852c): undefined reference to `vm_commit_limit'


This patch added vm_commit_limit() to mm/mmap.c, but no-mmu systems
use mm/nommu.c instead.

commit 50e05119f980037b474043434cb713bf6f859734
Author: Andrew Morton <akpm@linux-foundation.org>
Date:   Thu Nov 7 12:01:09 2013 +1100

    mm-factor-commit-limit-calculation-fix
    
    uninline vm_commit_limit()

Re: linux-next: build failure

[J.R. Mauro]

On Mon, Oct 20, 2008 at 01:47:50PM -0700, Greg KH wrote:
> On Mon, Oct 20, 2008 at 09:51:42AM -0700, Greg KH wrote:
> > On Mon, Oct 20, 2008 at 12:33:29PM -0400, J.R. Mauro wrote:
> > > On Mon, Oct 20, 2008 at 11:48 AM, Greg KH <greg@kroah.com> wrote:
> > > > On Mon, Oct 20, 2008 at 11:04:20PM +1100, Stephen Rothwell wrote:
> > > >> Hi Greg,
> > > >>
> > > >> Today's linux-next build (powerpc allyesconfig) failed like this:
> > > >>
> > > >> In file included from drivers/staging/sxg/sxg.c:77:
> > > >> drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
> > > >> drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
> > > >> drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
> > > >> drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
> > > >> drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
> > > >> drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
> > > >> drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'
> > > >>
> > > >> I applied the following patch.
> > > >
> > > > Ah, doh, sorry, I should have caught that.  I'll go apply this to the
> > > > tree, thanks.
> > > 
> > > Greg,
> > > 
> > > The slicoss driver also from Alacritech looks like it needs a depends
> > > on X86 as well. Should the sxg and slicoss drivers from Alacritech be
> > > doing things like this (sxg/sxghif.h:851, slicoss/slicoss.c:960):
> > 
> > Yes, it needs it too.
> 
> Hm, it already has this dependancy.  In drivers/staging/slicoss/Kconfig
> we have:
> 	config SLICOSS
> 		tristate "Alacritech Gigabit IS-NIC support"
> 		depends on PCI && X86 && NETDEV_1000
> 
> So we should be fine here.

Sorry about that, had to pull from Linus's tree

> 
> thanks,
> 
> greg k-h

Re: linux-next: build failure

[Stephen Rothwell]

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

Hi Greg,

On Mon, 20 Oct 2008 08:48:57 -0700 Greg KH <greg@kroah.com> wrote:
>
> Ah, doh, sorry, I should have caught that.  I'll go apply this to the
> tree, thanks.

Linus seems to have already applied it.

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

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

Re: linux-next: build failure

[Greg KH]

On Mon, Oct 20, 2008 at 09:51:42AM -0700, Greg KH wrote:
> On Mon, Oct 20, 2008 at 12:33:29PM -0400, J.R. Mauro wrote:
> > On Mon, Oct 20, 2008 at 11:48 AM, Greg KH <greg@kroah.com> wrote:
> > > On Mon, Oct 20, 2008 at 11:04:20PM +1100, Stephen Rothwell wrote:
> > >> Hi Greg,
> > >>
> > >> Today's linux-next build (powerpc allyesconfig) failed like this:
> > >>
> > >> In file included from drivers/staging/sxg/sxg.c:77:
> > >> drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
> > >> drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
> > >> drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
> > >> drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
> > >> drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
> > >> drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
> > >> drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'
> > >>
> > >> I applied the following patch.
> > >
> > > Ah, doh, sorry, I should have caught that.  I'll go apply this to the
> > > tree, thanks.
> > 
> > Greg,
> > 
> > The slicoss driver also from Alacritech looks like it needs a depends
> > on X86 as well. Should the sxg and slicoss drivers from Alacritech be
> > doing things like this (sxg/sxghif.h:851, slicoss/slicoss.c:960):
> 
> Yes, it needs it too.

Hm, it already has this dependancy.  In drivers/staging/slicoss/Kconfig
we have:
	config SLICOSS
		tristate "Alacritech Gigabit IS-NIC support"
		depends on PCI && X86 && NETDEV_1000

So we should be fine here.

thanks,

greg k-h

Re: linux-next: build failure

[Greg KH]

On Mon, Oct 20, 2008 at 12:33:29PM -0400, J.R. Mauro wrote:
> On Mon, Oct 20, 2008 at 11:48 AM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Oct 20, 2008 at 11:04:20PM +1100, Stephen Rothwell wrote:
> >> Hi Greg,
> >>
> >> Today's linux-next build (powerpc allyesconfig) failed like this:
> >>
> >> In file included from drivers/staging/sxg/sxg.c:77:
> >> drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
> >> drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
> >> drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
> >> drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
> >> drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
> >> drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
> >> drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'
> >>
> >> I applied the following patch.
> >
> > Ah, doh, sorry, I should have caught that.  I'll go apply this to the
> > tree, thanks.
> 
> Greg,
> 
> The slicoss driver also from Alacritech looks like it needs a depends
> on X86 as well. Should the sxg and slicoss drivers from Alacritech be
> doing things like this (sxg/sxghif.h:851, slicoss/slicoss.c:960):

Yes, it needs it too.

thanks,

greg k-h

Re: linux-next: build failure

[J.R. Mauro]

On Mon, Oct 20, 2008 at 11:48 AM, Greg KH <greg@kroah.com> wrote:
> On Mon, Oct 20, 2008 at 11:04:20PM +1100, Stephen Rothwell wrote:
>> Hi Greg,
>>
>> Today's linux-next build (powerpc allyesconfig) failed like this:
>>
>> In file included from drivers/staging/sxg/sxg.c:77:
>> drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
>> drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
>> drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
>> drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
>> drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
>> drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
>> drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'
>>
>> I applied the following patch.
>
> Ah, doh, sorry, I should have caught that.  I'll go apply this to the
> tree, thanks.

Greg,

The slicoss driver also from Alacritech looks like it needs a depends
on X86 as well. Should the sxg and slicoss drivers from Alacritech be
doing things like this (sxg/sxghif.h:851, slicoss/slicoss.c:960):

#if defined(CONFIG_X86_64)

...

#else
    Stop Compilation;
#endif


>
> greg k-h
> --
> 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/
>

Re: linux-next: build failure

[Greg KH]

On Mon, Oct 20, 2008 at 11:04:20PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next build (powerpc allyesconfig) failed like this:
> 
> In file included from drivers/staging/sxg/sxg.c:77:
> drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
> drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
> drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
> drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
> drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
> drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
> drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'
> 
> I applied the following patch.

Ah, doh, sorry, I should have caught that.  I'll go apply this to the
tree, thanks.

greg k-h

linux-next: build failure

[Stephen Rothwell]

Hi Greg,

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

In file included from drivers/staging/sxg/sxg.c:77:
drivers/staging/sxg/sxghif.h:859: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'Compilation'
drivers/staging/sxg/sxg.c: In function 'sxg_transmit_packet':
drivers/staging/sxg/sxg.c:1949: error: 'SXG_SGL_BUF_SIZE' undeclared (first use in this function)
drivers/staging/sxg/sxg.c:1958: error: implicit declaration of function 'SXG_SGL_BUFFER'
drivers/staging/sxg/sxg.c:1958: warning: assignment makes pointer from integer without a cast
drivers/staging/sxg/sxg.c: In function 'sxg_mcast_set_mask':
drivers/staging/sxg/sxg.c:2846: warning: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'u64'

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, 20 Oct 2008 22:59:18 +1100
Subject: [PATCH] staging: sxg depends on X86

sxghif.h has code that explicitly will not build fo other architecures.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/staging/sxg/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/sxg/Kconfig b/drivers/staging/sxg/Kconfig
index 1ae3508..6e6cf0b 100644
--- a/drivers/staging/sxg/Kconfig
+++ b/drivers/staging/sxg/Kconfig
@@ -1,6 +1,7 @@
 config SXG
 	tristate "Alacritech SLIC Technology Non-Accelerated 10Gbe support"
 	depends on PCI && NETDEV_10000
+	depends on X86
 	default n
 	help
 	  This driver supports the Alacritech SLIC Technology Non-Accelerated
-- 
1.5.6.5

Re: linux-next: build failure

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 25 Aug 2008 21:28:33 +1000

> Hi Ingo,
> 
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
> ERROR: "__BUILD_BUG_ON_non_constant" [drivers/net/sunvnet.ko] undefined!
> ERROR: "__BUILD_BUG_ON_non_constant" [drivers/block/sunvdc.ko] undefined!
> 
> Probably intorduced by commit f5b5d41dd51a31fe70e3a04fb80a3b90b84c6a4e
> ("debug: fix BUILD_BUG_ON() for non-constant expressions").

I think __builtin_constant_p() is not seeing something it
should here.

It isn't accepting:

--------------------
static inline __attribute__((const))
bool is_power_of_2(unsigned long n)
{
	return (n != 0 && ((n & (n - 1)) == 0));
}
--------------------

and thus is_power_of_2(256) as being a constant.

If a 'const' inline function being passed a const argument isn't
constant, what is! :-)

I bet the problem is the fact that is_power_of_2() is a function.

I did some tests and I can only trigger this problem with gcc-3.4 on
sparc, gcc-4.1 and gcc-4.2 worked fine.

It triggers with both -O2 and -Os for the following simple test case:

static inline __attribute__((const))
int is_power_of_2(unsigned long n)
{
	return (n != 0 && ((n & (n - 1)) == 0));
}

extern int bar;

int main(void)
{
	if (!__builtin_constant_p(is_power_of_2(256)))
		bar++;
	return 0;
}

davem@sunset:~/src/GIT/linux-2.6$ gcc-3.4  -Os -o x x.c
/tmp/ccy8FzD8.o: In function `main':
x.c:(.text+0x0): undefined reference to `bar'
x.c:(.text+0x4): undefined reference to `bar'
x.c:(.text+0xc): undefined reference to `bar'
collect2: ld returned 1 exit status
davem@sunset:~/src/GIT/linux-2.6$ gcc-3.4  -O2 -o x x.c
/tmp/ccQNzZuj.o: In function `main':
x.c:(.text+0x0): undefined reference to `bar'
x.c:(.text+0x4): undefined reference to `bar'
x.c:(.text+0xc): undefined reference to `bar'
collect2: ld returned 1 exit status
davem@sunset:~/src/GIT/linux-2.6$ 

Re: linux-next: build failure

[Ingo Molnar]

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

> Hi Ingo,
> 
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
> ERROR: "__BUILD_BUG_ON_non_constant" [drivers/net/sunvnet.ko] undefined!
> ERROR: "__BUILD_BUG_ON_non_constant" [drivers/block/sunvdc.ko] undefined!
> 
> Probably intorduced by commit f5b5d41dd51a31fe70e3a04fb80a3b90b84c6a4e
> ("debug: fix BUILD_BUG_ON() for non-constant expressions").
> 
> The preprocessed code looks like this:
> 
> static inline __attribute__((always_inline)) u32 vio_dring_avail(struct vio_dring_state *dr,
>       unsigned int ring_size)
> {
>  do { (void)sizeof(char[1 - 2*!!(!is_power_of_2(ring_size))]); if (!__builtin_constant_p(!is_power_of_2(ring_size))) __BUILD_BUG_ON_non_constant++; } while (0);
> 
>  return (dr->pending -
>   ((dr->prod - dr->cons) & (ring_size - 1)));
> }
> 
> I tried turning the above inline function into a macro to no avail.
> 
> I applied the following patch (which is probably not what is wanted, 
> but puts back what was there before).
> 
> I see from LKML that this definition of BUILD_BUG_ON is to be 
> replaced, so this is just a temporary measure.

yeah. Note that it will only be stricter, so more fallout is expected.

	Ingo

linux-next: build failure

[Stephen Rothwell]

Hi Ingo,

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

ERROR: "__BUILD_BUG_ON_non_constant" [drivers/net/sunvnet.ko] undefined!
ERROR: "__BUILD_BUG_ON_non_constant" [drivers/block/sunvdc.ko] undefined!

Probably intorduced by commit f5b5d41dd51a31fe70e3a04fb80a3b90b84c6a4e
("debug: fix BUILD_BUG_ON() for non-constant expressions").

The preprocessed code looks like this:

static inline __attribute__((always_inline)) u32 vio_dring_avail(struct vio_dring_state *dr,
      unsigned int ring_size)
{
 do { (void)sizeof(char[1 - 2*!!(!is_power_of_2(ring_size))]); if (!__builtin_constant_p(!is_power_of_2(ring_size))) __BUILD_BUG_ON_non_constant++; } while (0);

 return (dr->pending -
  ((dr->prod - dr->cons) & (ring_size - 1)));
}

I tried turning the above inline function into a macro to no avail.

I applied the following patch (which is probably not what is wanted, but
puts back what was there before).

I see from LKML that this definition of BUILD_BUG_ON is to be replaced,
so this is just a temporary measure.

-- 
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 21:16:14 +1000
Subject: [PATCH] revert BUILD_BUG_ON change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 include/linux/compiler.h |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d7d313b..a1c082d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -206,15 +206,9 @@ extern void __chk_io_ptr(const volatile void __iomem *);
  * ( The linker trick relies on gcc optimizing out a multiplication with
  *   constant zero - which should be reasonable enough. )
  */
-#ifndef __ASSEMBLY__
-extern unsigned int __BUILD_BUG_ON_non_constant;
-#endif
-
 #define BUILD_BUG_ON(condition)					\
 do {								\
 	(void)sizeof(char[1 - 2*!!(condition)]);		\
-	if (!__builtin_constant_p(condition))			\
-		__BUILD_BUG_ON_non_constant++;			\
 } while (0)
 
 /*
-- 
1.5.6.3

Re: linux-next: build failure

[Linus Torvalds]

On Tue, 29 Jul 2008, Linus Torvalds wrote:
>
>   [...]                           since statement expressions are gcc 
> extensions, and as such the gcc people could make up any semantics they 
> want to them, including just defining that a statement expression with 
> an lvalue value is the same lvalue rather than any temporary). 

In fact, that does seem what gcc-4.x does. The way to tell is to do

	const int *x;

	({ *x }) = 1;

and it's (a) legal (assignments to non-lvalues wouldn't work) and (b) 
gives a nice warning about assignment to read-only location, which in turn 
implies that the compiler properly just peeled off the de-reference even 
though it was inside the statement expression.

IOW, at least in gcc-4.3 (and apparently in earlier gcc-4 versions, but 
not in gcc-3.4.5), a statement expression with an lvalue return value _is_ 
actually an lvalue.

But that also means that there is no difference what-so-ever between (x) 
and ({ x; }) in gcc-4. And in gcc-3 there is, because apparently in gcc-3 
a statement expression is never an lvalue (which is actually the sane 
thing, imho).

			Linus

Re: linux-next: build failure

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> In contrast, "address-of lvalue" is _guaranteed_ to not do anything 
> stupid like that, and gives just the address-of.
> 
> Oh, and I was wrong about the &*x losing the 'const'. It doesn't. So I 
> think Stephen's patch is fine after all - if somebody tries to modify 
> the end result through the pointer, it will give a big compiler 
> warning.

yeah, both variants do that, i've checked it earlier today - i tried to 
find a way to get something more drastic than a compiler warning. (but 
failed)

	Ingo

Re: linux-next: build failure

[Linus Torvalds]

On Tue, 29 Jul 2008, Ingo Molnar wrote:
> 
> the fix is simple enough.
> 
> but the question is, wont it generate huge artificial stackframes with 
> CONFIG_MAXSMP and NR_CPUS=4096?

Quite the reverse.

The "address-of statement expression" is the one that is more likely to 
generate artificial stack-frames because of a temporary variable (of 
course, I wouldn't count on it, since statement expressions are gcc 
extensions, and as such the gcc people could make up any semantics they 
want to them, including just defining that a statement expression with 
an lvalue value is the same lvalue rather than any temporary). 

In contrast, "address-of lvalue" is _guaranteed_ to not do anything stupid 
like that, and gives just the address-of.

Oh, and I was wrong about the &*x losing the 'const'. It doesn't. So I 
think Stephen's patch is fine after all - if somebody tries to modify the 
end result through the pointer, it will give a big compiler warning.

			Linus

Re: linux-next: build failure

[Linus Torvalds]

On Tue, 29 Jul 2008, Ingo Molnar wrote:
>
> > @@ -287,7 +287,7 @@ static inline const cpumask_t *get_cpu_mask(unsigned int cpu)
> >   * gcc optimizes it out (it's a constant) and there's no huge stack
> >   * variable created:
> >   */
> > -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> > +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> 
> hm, i'm wondering - is this a compiler bug?

Not necessarily. The code is fragile.

Doing a statement expression basically creates a new temporary variable 
with pretty much undefined scope. Taking the address of it *may* be 
optimized away to the original cpu_mask pointer, but it's not really a 
well-defined operation: there really _is_ a local temporary variable, and 
if you were to change things through the address-of thing, gcc would be 
buggy if it had done the optimization.

So the "address-of statement expression" really is a dubious construct.

That said, the change that Stephen introduces is _not_ a no-op. It very 
fundamentally changes the code - exactly because now there is no temporary 
value created any more: it's a real lvalue, and now anybody who does 
&cpumask_of_cpu(cpu) will fundamentally get the original pointer back, 
except it has lost the "const".

And that's actually dangerous - exactly because it now loses the "const" 
thing, there may be people who end up modifying the result without 
getting any compile-time warnings.

I would _seriously_ suggest that both the original code and Stephen's 
modified version is really bad. And you should have taken my interface 
as-is - one that returns a "const cpumask_t *", and nothing else. 

Anything else is simply fundamentally broken.

			Linus

Re: linux-next: build failure

[Mike Travis]

Mike Travis wrote:
> Ingo Molnar wrote:
>> * KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
>>
>>>> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>
>>>>> Hi Ingo,
>>>>>
>>>>> On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>>>>>>> -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
>>>>>>> +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
>>>>>> hm, i'm wondering - is this a compiler bug?
>>>>> Or maybe a deficiency in such an old compiler (v3.4.5) but the fix 
>>>>> makes sense anyway, right?
>>>> yeah, i was just wondering.
>>> in linux/README
>>>
>>> COMPILING the kernel:
>>>
>>>  - Make sure you have at least gcc 3.2 available.
>>>    For more information, refer to Documentation/Changes.
>>>
>>> So, if 3.4.5 is old, Should we change readme?
>> the fix is simple enough.
>>
>> but the question is, wont it generate huge artificial stackframes with 
>> CONFIG_MAXSMP and NR_CPUS=4096? Maybe it is unable to figure out and 
>> simplify the arithmetics there - or something like that.
>>
>> 	Ingo
> 
> I've looked at stack frames quite extensively and usually they are
> not generated unless you explicitly use a named cpumask variable,
> pass a cpumask by value, expect a cpumask function return, create
> an initializer that contains a cpumask field, and (probably a couple
> more I missed).
> 
> Almost all others are done efficiently via pointers or simple
> struct copies:
> 
> 	cpus_xxx(*cpumask_of_cpu(), ...
> 	struct->cpumask_var = *cpumask_of_cpu()
> 	global_cpumask_var = *cpumask_of_cpu()
> 	etc.
> 
> Thanks,
> Mike

Geez, I edited the above after I first used *cpumask_var and didn't
get the semantics right!

 	cpus_xxx(cpumask_of_cpu(), ...
 	struct->cpumask_var = cpumask_of_cpu()
 	global_cpumask_var = cpumask_of_cpu()

Re: linux-next: build failure

[Mike Travis]

Ingo Molnar wrote:
> * KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
>>> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>
>>>> Hi Ingo,
>>>>
>>>> On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>>>>>> -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
>>>>>> +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
>>>>> hm, i'm wondering - is this a compiler bug?
>>>> Or maybe a deficiency in such an old compiler (v3.4.5) but the fix 
>>>> makes sense anyway, right?
>>> yeah, i was just wondering.
>> in linux/README
>>
>> COMPILING the kernel:
>>
>>  - Make sure you have at least gcc 3.2 available.
>>    For more information, refer to Documentation/Changes.
>>
>> So, if 3.4.5 is old, Should we change readme?
> 
> the fix is simple enough.
> 
> but the question is, wont it generate huge artificial stackframes with 
> CONFIG_MAXSMP and NR_CPUS=4096? Maybe it is unable to figure out and 
> simplify the arithmetics there - or something like that.
> 
> 	Ingo

I've looked at stack frames quite extensively and usually they are
not generated unless you explicitly use a named cpumask variable,
pass a cpumask by value, expect a cpumask function return, create
an initializer that contains a cpumask field, and (probably a couple
more I missed).

Almost all others are done efficiently via pointers or simple
struct copies:

	cpus_xxx(*cpumask_of_cpu(), ...
	struct->cpumask_var = *cpumask_of_cpu()
	global_cpumask_var = *cpumask_of_cpu()
	etc.

Thanks,
Mike

Re: linux-next: build failure

[Ingo Molnar]

* KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > 
> > * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > 
> > > Hi Ingo,
> > > 
> > > On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> > > >
> > > > > -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> > > > > +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> > > > 
> > > > hm, i'm wondering - is this a compiler bug?
> > > 
> > > Or maybe a deficiency in such an old compiler (v3.4.5) but the fix 
> > > makes sense anyway, right?
> > 
> > yeah, i was just wondering.
> 
> in linux/README
> 
> COMPILING the kernel:
> 
>  - Make sure you have at least gcc 3.2 available.
>    For more information, refer to Documentation/Changes.
> 
> So, if 3.4.5 is old, Should we change readme?

the fix is simple enough.

but the question is, wont it generate huge artificial stackframes with 
CONFIG_MAXSMP and NR_CPUS=4096? Maybe it is unable to figure out and 
simplify the arithmetics there - or something like that.

	Ingo

Re: linux-next: build failure

[KOSAKI Motohiro]

> 
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi Ingo,
> > 
> > On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> > >
> > > > -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> > > > +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> > > 
> > > hm, i'm wondering - is this a compiler bug?
> > 
> > Or maybe a deficiency in such an old compiler (v3.4.5) but the fix 
> > makes sense anyway, right?
> 
> yeah, i was just wondering.

in linux/README

COMPILING the kernel:

 - Make sure you have at least gcc 3.2 available.
   For more information, refer to Documentation/Changes.



So, if 3.4.5 is old, Should we change readme?

Re: linux-next: build failure

[Ingo Molnar]

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

> Hi Ingo,
> 
> On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > > -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> > > +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> > 
> > hm, i'm wondering - is this a compiler bug?
> 
> Or maybe a deficiency in such an old compiler (v3.4.5) but the fix 
> makes sense anyway, right?

yeah, i was just wondering.

	Ingo

Re: linux-next: build failure

[Wenji Huang]

Ingo Molnar wrote:
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
>> Hi Dave,
>>
>> Today's linux-next build (sparc64 defconfig) failed like this:
>>
>> kernel/time/tick-common.c: In function `tick_check_new_device':
>> kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
>> kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
>> kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
>>
>> gcc is version 3.4.5 sparc64 cross compiler (powercp64 host).
>>
>> The below patch fixes it.
>>
>> when you take the address of the result.  Noticed on a sparc64 compile
>> using a version 3.4.5 cross compiler.
>>
>> kernel/time/tick-common.c: In function `tick_check_new_device':
>> kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
>> kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
>> kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
>>
>> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> ---
>>  include/linux/cpumask.h |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
>> index 96d0509..d3219d7 100644
>> --- a/include/linux/cpumask.h
>> +++ b/include/linux/cpumask.h
>> @@ -287,7 +287,7 @@ static inline const cpumask_t *get_cpu_mask(unsigned int cpu)
>>   * gcc optimizes it out (it's a constant) and there's no huge stack
>>   * variable created:
>>   */
>> -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
>> +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> 
> hm, i'm wondering - is this a compiler bug?
> 
> 	Ingo

Same problem on x86/gcc 3.4.6, but will pass on gcc 4.x

Regards,
Wenji

Re: linux-next: build failure

[Stephen Rothwell]

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

Hi Ingo,

On Tue, 29 Jul 2008 10:00:55 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>
> > -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> > +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> 
> hm, i'm wondering - is this a compiler bug?

Or maybe a deficiency in such an old compiler (v3.4.5) but the fix makes
sense anyway, right?

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

[Ingo Molnar]

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

> Hi Dave,
> 
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
> kernel/time/tick-common.c: In function `tick_check_new_device':
> kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
> kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
> kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
> 
> gcc is version 3.4.5 sparc64 cross compiler (powercp64 host).
> 
> The below patch fixes it.
>
> when you take the address of the result.  Noticed on a sparc64 compile
> using a version 3.4.5 cross compiler.
> 
> kernel/time/tick-common.c: In function `tick_check_new_device':
> kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
> kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
> kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  include/linux/cpumask.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 96d0509..d3219d7 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -287,7 +287,7 @@ static inline const cpumask_t *get_cpu_mask(unsigned int cpu)
>   * gcc optimizes it out (it's a constant) and there's no huge stack
>   * variable created:
>   */
> -#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
> +#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))

hm, i'm wondering - is this a compiler bug?

	Ingo

linux-next: build failure

[Stephen Rothwell]

Hi Dave,

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

kernel/time/tick-common.c: In function `tick_check_new_device':
kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'

gcc is version 3.4.5 sparc64 cross compiler (powercp64 host).

The below patch fixes it.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 29 Jul 2008 16:07:37 +1000
Subject: [PATCH] cpumask: statement expressions confuse some versions of gcc

when you take the address of the result.  Noticed on a sparc64 compile
using a version 3.4.5 cross compiler.

kernel/time/tick-common.c: In function `tick_check_new_device':
kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'

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

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 96d0509..d3219d7 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -287,7 +287,7 @@ static inline const cpumask_t *get_cpu_mask(unsigned int cpu)
  * gcc optimizes it out (it's a constant) and there's no huge stack
  * variable created:
  */
-#define cpumask_of_cpu(cpu) ({ *get_cpu_mask(cpu); })
+#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
 
 
 #define CPU_MASK_LAST_WORD BITMAP_LAST_WORD_MASK(NR_CPUS)
-- 
1.5.6.3

linux-next: build failure

[Stephen Rothwell]

Hi James, Andrew,

(Previously reported by Dave Miller.)

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

drivers/scsi/qla2xxx/qla_attr.c: In function 'qla24xx_vport_delete':
drivers/scsi/qla2xxx/qla_attr.c:1184: error: implicit declaration of function 'msleep'

I applied the patch below.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

>From 879b13261ee593b8afcae80a33844f162e8e8a5c Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 28 Jul 2008 15:02:35 +1000
Subject: [PATCH] scsi: msleep is declared in delay.h

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/scsi/qla2xxx/qla_attr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 7a4409a..a319a20 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -8,6 +8,7 @@
 
 #include <linux/kthread.h>
 #include <linux/vmalloc.h>
+#include <linux/delay.h>
 
 static int qla24xx_vport_disable(struct fc_vport *, bool);
 
-- 
1.5.6.3

linux-next: build failure

[Stephen Rothwell]

Hi Karsten,

(Previously reported by Dave Miller for Sparc)

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

drivers/isdn/hardware/mISDN/hfcmulti.c:5255:2: error: #error "not running on big endian machines now"

I just disabled it using the patch below.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

>From c4f3c0cba3811f71465c332fe36fe30b3b17f507 Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 28 Jul 2008 14:38:29 +1000
Subject: [PATCH] hfcmulti is not supported on big endian

So stick it to X86 for now.

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

diff --git a/drivers/isdn/hardware/mISDN/Kconfig b/drivers/isdn/hardware/mISDN/Kconfig
index 9cd5f5f..1a78575 100644
--- a/drivers/isdn/hardware/mISDN/Kconfig
+++ b/drivers/isdn/hardware/mISDN/Kconfig
@@ -16,6 +16,7 @@ config MISDN_HFCMULTI
 	tristate "Support for HFC multiport cards (HFC-4S/8S/E1)"
 	depends on PCI
 	depends on MISDN
+	depends on X86
 	help
 	  Enable support for cards with Cologne Chip AG's HFC multiport
 	  chip. There are three types of chips that are quite similar,
-- 
1.5.6.3