__linux__ is not getting defined in android-build

[Zach Pfeffer]

On 5 October 2011 09:03, Andy Green <email address hidden> wrote:
> On 10/05/2011 09:54 PM, Somebody in the thread at some point said:
>>
>> On 5 October 2011 08:39, Andy Green<email address hidden>  wrote:
>
>>> #else /* One of the BSDs */
>>>
>>>
>>> #include<sys/ioccom.h>
>>>
>>> I assume you don't mean to be building for BSD.
>>>
>>> It looks like the way you build the kernel is broken.
>>
>> Hmm... We've got:
>>
>>        make ARCH=arm
>> CROSS_COMPILE=/mnt/jenkins/workspace/linaro-android_tracking-panda/build/toolchain/bin/arm-eabi-
>> defconfig android_omap4_defconfig&&\
>>        make ARCH=arm
>> CROSS_COMPILE=/mnt/jenkins/workspace/linaro-android_tracking-panda/build/toolchain/bin/arm-eabi-
>> uImage&&\
>>        make ARCH=arm
>> CROSS_COMPILE=/mnt/jenkins/workspace/linaro-android_tracking-panda/build/toolchain/bin/arm-eabi-
>> modules
>>
>> wouldn't __linux__ be defined in the kernel build system?
>
> Yes, and your make lines look reasonable.
>
> Is this perhaps coming from broken toolchain and / or binutils then? Didn't
> I read you are using HEAD toolchain pieces from somewhere now?
>
> There are several places in kernel that use code coming from BSD or designed
> to be 'dual use' that uses __linux__ like this, and it all works fine on my
> 4.5 stuff.  Sebjan cooks the same trees using 4.6 gcc ok... it can be
> binutils or make issue too I guess.
>
> For reference my setup is
>
> binutils-2.20.51.0.12-2
> gcc-4.5.1-5
> make-3.82-6
>
> Your best bet, if it's true you are using avant garde toolchain, is bounce
> it off toolchain guys, there's nothing to fix in our tree as it stands.

Yup. We're one the latest and greatest from Linaro. Its always listed on the build page at:

https://android-build.linaro.org/builds/~linaro-android/tracking-panda/

I've tried both:

build 3:

https://android-build.linaro.org/jenkins/job/linaro-android_toolchain-4.6-bzr/33/artifact/build/out/android-toolchain-eabi-4.6-33-2011-10-03_00-11-15-linux-x86.tar.bz2

build 4:

https://android-build.linaro.org/jenkins/job/linaro-android_toolchain-4.6-2011.09/1/artifact/build/out/android-toolchain-eabi-linaro-4.6-2011.09-1-1-2011-09-16_16-16-01-linux-x86.tar.bz2

(Linux/GNU Binutils) 2.21.53.0.2.20110804
gcc version 4.6.2 20110908

...for make we're at Natty Narwhal at the moment.

Bero or Ken do you guys know where __linux__ gets defined in the build chain?

Is this a regression? Bero, would you mind taking a look?

> -Andy
>
> --
> Andy Green | TI Landing Team Leader
> Linaro.org │ Open source software for ARM SoCs | Follow Linaro
> http://facebook.com/pages/Linaro/155974581091106  -
> http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
>

--
Zach Pfeffer
Android Platform Team Lead, Linaro Platform Teams
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

[Alexander Sack (asac)]

My believe is that a bare metal toolchain typically does not define __linux__ ... my gut feel is
that this is a kernel code regression.

Andy, can you check if the #ifdef __linux__ code was introduced after our last 3.0 kernel
drop that we are working with?

[Alexander Sack (asac)]

here the patch that paul did for 3.0 and that andy applied now to workaround:

Date: Fri, 3 Jun 2011 18:04:45 +0300
From: Paul Sokolovsky <email address hidden>
To: linaro-dev <email address hidden>, <email address hidden>,
Alexander Sack <email address hidden> Subject: [PATCH] drm.h: Fix DRM
compilation with bare-metal toolchain.

An ifdef in drm.h expects to be compiled with full-fledged Linux
toolchain, but it's common to compile kernel with just bare-metal
toolchain which doesn't define __linux__. So, also add __KERNEL__
check.

Signed-off-by: Paul Sokolovsky <email address hidden>
---
 include/drm/drm.h | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4be33b4..45435e3 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -36,7 +36,7 @@
 #ifndef _DRM_H_
 #define _DRM_H_

-#if defined(__linux__)
+#if defined(__KERNEL__) || defined(__linux__)

 #include <linux/types.h>
 #include <asm/ioctl.h>
--
1.7.1

[Alexander Sack (asac)]

here the error message we get when building with bare-metal:

target thumb C++: libwebcore <= external/webkit/WebCore/rendering/MediaControlElements.cpp
/mnt/jenkins/workspace/linaro-android_tracking-panda/build/toolchain/bin/arm-eabi-g++ -I bionic -I external/stlport/stlport -I dalvik/libnativehelper/include/nativehelper -I external/webkit/WebKit/android/icu -I external/ -I external/icu4c/common -I external/icu4c/i18n -I external/libxml2/include -I external/skia/emoji -I external/skia/include/core -I external/skia/include/effects -I external/skia/include/images -I external/skia/include/ports -I external/skia/include/utils -I external/skia/src/ports -I external/sqlite/dist -I frameworks/base/core/jni/android/graphics -I external/webkit/WebCore -I external/webkit/WebCore/accessibility -I external/webkit/WebCore/bindings/generic -I external/webkit/WebCore/css -I external/webkit/WebCore/dom -I external/webkit/WebCore/editing -I external/webkit/WebCore/history -I external/webkit/WebCore/history/android -I external/webkit/WebCore/html -I external/webkit/WebCore/html/canvas -I external/webkit/WebCore/inspector -I external/webkit/WebCore/loader -I external/webkit/WebCore/loader/appcache -I external/webkit/WebCore/loader/icon -I external/webkit/WebCore/notifications -I external/webkit/WebCore/page -I external/webkit/WebCore/page/android -I external/webkit/WebCore/page/animation -I external/webkit/WebCore/platform -I external/webkit/WebCore/platform/android -I external/webkit/WebCore/platform/animation -I external/webkit/WebCore/platform/graphics -I external/webkit/WebCore/platform/graphics/android -I external/webkit/WebCore/platform/graphics/filters -I external/webkit/WebCore/platform/graphics/network -I external/webkit/WebCore/platform/graphics/skia -I external/webkit/WebCore/platform/graphics/transforms -I external/webkit/WebCore/platform/image-decoders -I external/webkit/WebCore/platform/image-decoders/bmp -I external/webkit/WebCore/platform/image-decoders/gif -I external/webkit/WebCore/platform/image-decoders/ico -I external/webkit/WebCore/platform/image-decoders/jpeg -I external/webkit/WebCore/platform/image-decoders/png -I external/webkit/WebCore/platform/mock -I external/webkit/WebCore/platform/network -I external/webkit/WebCore/platform/network/android -I external/webkit/WebCore/platform/sql -I external/webkit/WebCore/platform/text -I external/webkit/WebCore/plugins -I external/webkit/WebCore/plugins/android -I external/webkit/WebCore/rendering -I external/webkit/WebCore/rendering/style -I external/webkit/WebCore/storage -I external/webkit/WebCore/svg -I external/webkit/WebCore/svg/animation -I external/webkit/WebCore/svg/graphics -I external/webkit/WebCore/svg/graphics/filters -I external/webkit/WebCore/websockets -I external/webkit/WebCore/workers -I external/webkit/WebCore/xml -I external/webkit/WebKit/android -I external/webkit/WebKit/android/WebCoreSupport -I external/webkit/WebKit/android/jni -I external/webkit/WebKit/android/nav -I external/webkit/WebKit/android/plugins -I external/webkit/Java...

[Alexander Sack (asac)]

oops ... obviously the g++ command was not related due to -j8

[Alexander Sack (asac)]

linaro-android build is fixed by andy applying workaround patch from above. added linux-linaro to continue this bug on the upstream side of things. assigned to dave who started leading the effort to fix this proper.

[Dave Martin (dave-martin-arm)]

Can someone give this a try?

For now, I've only attempted to touch headers that look like
they are expoerted to userspace.

The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:

  Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)

are available in the git repository at:
  git://git.linaro.org/people/dmart/linux-2.6-arm.git linux/ifdef-kernel+for-linaro-android

Dave Martin (5):
      kbuild: Introduce -D__linux_kernel__ to indicate building of the Linux kernel
      acpi: avoid #ifdef __linux__ misuse in exportable headers
      drm: avoid #ifdef __linux__ misuse in exportable headers
      coda: avoid #ifdef __linux__ misuse in exportable headers
      ext2: avoid #ifdef __linux__ misuse in exportable headers

 Makefile | 2 +-
 include/acpi/platform/acenv.h | 2 +-
 include/drm/drm.h | 2 +-
 include/linux/coda.h | 8 ++++----
 include/linux/ext2_fs.h | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

[Alexander Sack (asac)]

+andy ... he owns the tilt android tree where we see the issue and have a
workaround atm.

On Thu, Oct 6, 2011 at 3:30 PM, Dave Martin <email address hidden> wrote:

> Can someone give this a try?
>
> For now, I've only attempted to touch headers that look like
> they are expoerted to userspace.
>
>
> The following changes since commit
> c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
>
> Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
>
> are available in the git repository at:
> git://git.linaro.org/people/dmart/linux-2.6-arm.gitlinux/ifdef-kernel+for-linaro-android
>
> Dave Martin (5):
> kbuild: Introduce -D__linux_kernel__ to indicate building of the Linux
> kernel
> acpi: avoid #ifdef __linux__ misuse in exportable headers
> drm: avoid #ifdef __linux__ misuse in exportable headers
> coda: avoid #ifdef __linux__ misuse in exportable headers
> ext2: avoid #ifdef __linux__ misuse in exportable headers
>
> Makefile | 2 +-
> include/acpi/platform/acenv.h | 2 +-
> include/drm/drm.h | 2 +-
> include/linux/coda.h | 8 ++++----
> include/linux/ext2_fs.h | 2 +-
> 5 files changed, 8 insertions(+), 8 deletions(-)
>

--
Alexander Sack
Technical Director, Linaro Platform Teams
http://www.linaro.org | Open source software for ARM SoCs
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

[Mans Rullgard (mansr)]

On 6 October 2011 15:16, Alexander Sack <email address hidden> wrote:
> +andy ... he owns the tilt android tree where we see the issue and have a
> workaround atm.
>
> On Thu, Oct 6, 2011 at 3:30 PM, Dave Martin <email address hidden> wrote:
>>
>> Can someone give this a try?
>>
>> For now, I've only attempted to touch headers that look like
>> they are expoerted to userspace.
>>
>>
>> The following changes since commit
>> c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
>>
>>  Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
>>
>> are available in the git repository at:
>>  git://git.linaro.org/people/dmart/linux-2.6-arm.git
>> linux/ifdef-kernel+for-linaro-android
>>
>> Dave Martin (5):
>>      kbuild: Introduce -D__linux_kernel__ to indicate building of the Linux kernel
>>      acpi: avoid #ifdef __linux__ misuse in exportable headers
>>      drm: avoid #ifdef __linux__ misuse in exportable headers
>>      coda: avoid #ifdef __linux__ misuse in exportable headers
>>      ext2: avoid #ifdef __linux__ misuse in exportable headers
>>
>>  Makefile                      |    2 +-
>>  include/acpi/platform/acenv.h |    2 +-
>>  include/drm/drm.h             |    2 +-
>>  include/linux/coda.h          |    8 ++++----
>>  include/linux/ext2_fs.h       |    2 +-
>>  5 files changed, 8 insertions(+), 8 deletions(-)

What about ext3_fs.h?

The mess in coda.h would get a bit more palatable if you also dropped
the provision for glibc < 2. I doubt anyone still uses that, so carrying
this ifdef around seems pointless. Perhaps that's better done in a
separate patch though.

The instances of __linux__ in .c files should probably all be replaced
by __linux_kernel__.

--
Mans Rullgard / mru

[Arnd Bergmann (arnd-arndb)]

On Thursday 06 October 2011, Dave Martin wrote:
> Can someone give this a try?
>
> For now, I've only attempted to touch headers that look like
> they are expoerted to userspace.

I've looked at the patch and found small typos:

-#if defined(__linux__)
+#if define (__linux_kernel__) || defined(__linux__)
      ^^^^^^

-#ifndef __KERNEL__
+#ifndef __liunx_kernel__
    ^^^^^

More importantly, it's not at all clear from the description why you are
even doing this. If the android toolchain does not set __linux__, this
sounds more like a bug in the android toolchain. I would expect much
more stuff to break because of that.

 Arnd

[Nicolas Pitre (npitre)]

On Fri, 7 Oct 2011, Arnd Bergmann wrote:

> More importantly, it's not at all clear from the description why you are
> even doing this. If the android toolchain does not set __linux__, this
> sounds more like a bug in the android toolchain. I would expect much
> more stuff to break because of that.

Some people claim that this symbol belongs to libc and/or the user space
environment. The fact that we compile the kernel with a hosted compiler
providing that symbol by default doesn't necessarily make it legit in
the kernel.

The kernel source code is self contained. In theory, we should be able
to compile the Linux kernel with a compiler targetting non-Linux bare
metal configurations, and in that case it is expected for the __linux__
symbol to not be defined.

[warmcat (andy-warmcat)]

I don't have any opinion on validity of __linux__, but fwiw I have Dave's patchset in TILT tracking for now. If __linux__ is okay in kernel Dave's patches have minimal impact on both linux and non-linux cases, if __linux_ is deemed dodgy due to making a dependency on some detail of toolchain libs then it also has minimal impact. And with them, we're able to use current autobuild toolchain.

[Arnd Bergmann (arnd-arndb)]

I wonder if this simpler patch would also work. The header files in question are all for kernel modules that are shared with BSD or other operating systems. However, we know that when we are exporting the header files for user space that they will be used on a linux system, so we can simply replace any "#ifdef __linux__" with unconditional code. When building the kernel, there is no problem anyway since we know that we are building the kernel and even if the toolchain does not define __linux__, we can pass it as extra CFLAGS.

[Dave Martin (dave-martin-arm)]

On Fri, Oct 7, 2011 at 6:26 PM, Arnd Bergmann <email address hidden> wrote:
> On Thursday 06 October 2011, Dave Martin wrote:
>> Can someone give this a try?
>>
>> For now, I've only attempted to touch headers that look like
>> they are expoerted to userspace.
>
> I've looked at the patch and found small typos:
>
> -#if defined(__linux__)
> +#if define (__linux_kernel__) || defined(__linux__)
>      ^^^^^^
>
> -#ifndef __KERNEL__
> +#ifndef __liunx_kernel__
>           ^^^^^

Good spots, thanks -- those patches were put together pretty hastily,
I'm afraid.

> More importantly, it's not at all clear from the description why you are
> even doing this. If the android toolchain does not set __linux__, this
> sounds more like a bug in the android toolchain. I would expect much
> more stuff to break because of that.

My view is that __linux__ means "being built with a toolchain which
targets a linux-hosted userspace environment". This macro has no
meaningful interpretation when building the kernel, but it is
meaningful when building build-time tools etc.

This has nothing whatever to do with the runtime environment inside
the kernel -- the kernel _itself_ determines that, which is why (in my
view) this should be indicated by a separate define in the kernel
Makefile.

There's no reason in general why the Linux kernel shouldn't be built
with a BSD-targeted or bare-metal compiler. There's may also be no
reason why the BSD kernel shouldn't be built with a Linux-targeted
compiler -- however, I'm not familiar with the BSD build process, so
that may not make so much sense.

Cheers
---Dave

[Dave Martin (dave-martin-arm)]

I'm still far from convinced that defining __linux__ explicitly is the right thing to do here -- this feels like exchanging a slightly broken situation for a different slightly broken situation which happens to fix our particular problem.

Since __linux__ describes the compiler's target environment, and since the kernel's target environment is fully defined by the kernel and has nothing to do with the compiler's target environment, it does feel like we should have a separate macro, defined by the kernel build system itself.

Filtering the exported headers based on is probably still a good idea. __linux__ still feels wrong for actually filtering the headers -- there's no special reason why I shouldn't build the kernel on another OS, but this shouldn't affect the generated headers which are, by definition, meant for use with a linux-targeted compiler. Maybe we need another macro for this.

I think the number of uses of the macro is small enough that the change is feasible to implement.

Any concerns about this?

[Arnd Bergmann (arnd-arndb)]

Don't you also have to define __linux__ when building the kernel with a freestanding gcc? A few architectures (e.g. mips and score) always set __linux__ from their arch specific Makefile, and I see nothing wrong with that.

Also, we currently have to use android specific patches already when building a kernel for android, so adding yet another patch in there to add -D__linux__ for the andrdoid toolchain sounds reasonable. For the exported header files, let's first see if the unifdef approach works. AFAIU, the real issue here is that you cannot build android user space with the header files exported from a regular kernel, and that should really be possible and not rely on any patches at all.

[Dave Martin (dave-martin-arm)]

I don't disupte that your suggestion can work, and there is
precedent.

(Question: does android support any of the arches force-define
__linux__? Part of the problem here is the existence of
multiple possible userspace environments on top of the same
kernel, which breaks the normal assumptions.)

The problem is that __linux__ existed before us, and we can't
therefore dictate what it means while being guaranteed not to
get into trouble.

When building the kernel, we have more control over the
namespace, so to a large extent we can get away with having
this meaning for __linux__. But for code shared with userspace,
we don't have that luxury.

> Also, we currently have to use android specific patches already when
> building a kernel for android, so adding yet another patch in there to
> add -D__linux__ for the andrdoid toolchain sounds reasonable. For the
> exported header files, let's first see if the unifdef approach works.
> AFAIU, the real issue here is that you cannot build android user space
> with the header files exported from a regular kernel, and that should
> really be possible and not rely on any patches at all.

The problem with __linux__ in exportable headers is that is
used to mean three independent things:

a) The code is applicable when building the Linux kernel only
   (and should not be exported to userspace)

b) The code is applicable to interfacing with the Linux kernel
   (as opposed to, say, BSD)

c) The code is applicable to conventional Linux userspace (as
   opposed to, say, android)

(c) is used to adapt to system dependent things such as what is
expected to appear in the standard headers. This may be
different between Linux and android userspace.

Now, we could say that:
 (a) == defined(__linux__) && defined(__KERNEL__)
 (b) == defined(__linux__)

But we can't then say
 (c) == defined(__linux__)

... beacuse meaning (b) appears different from meaning (c). It
could be argued that (b) is necessary for (c), but (b) is not
sufficient for (c).

Unfortunatekly, (c) == defined(__linux__) is the de facto
meaning as defined by the toolchain and used all over userspace.
Therefore the condition for (b) has to be something less
specific. Because the definition given above is already
atomic, a less specific definition would have to look something
like:

 (b) == defined(__linux__) || defined(__something_else__)

If we have to invent __something_else__ in order to define (b),
then we may as well use it in the definition of (a) too,
instead of relying on __linux__. This would also improve
consistency, since it allows for a single, consistent meaning
both for __linux__ and for __something_else__, avoiding the
problem of having different meanings in different places.

That's the essence of why I think a new macro is needed to solve
the problem properly. If we must change stuff, I prefer to
improve the overall level of correctness and so reduce the
chance of having to revisit this in the future.

Mans' proposed __linux_kernel__ satisfies the requirements for
__something_else__ in this logic if we modify it not to imply
__KERNEL__, leading to:

 (a) == defined(__linux_kernel__) && defined(__KERNEL__)
 (b) == defined(__...

[Arnd Bergmann (arnd-arndb)]

My point here is that #ifdef __linux__ in header files exported from the Linux kernel is always bogus: We know that anything including this header is building code that will run on a Linux kernel, and we know that the kernel headers must not care if they are being included from glibc, uclibc, klibc or bionic.

The only correct answer for this is remove all conditionals that depend on __linux__ within exported header files and make those uncondition (or remove them when they depend on !defined(__linux__)).

[Dave Martin (dave-martin-arm)]

OK, now I think I understand your point of view.

If Linux kernel headers are not allowed to care at all what other non-
kernel headers may exist, then you are right, __linux__ should not be
referenced by the exported headers after unifdef processing.

This does open the way for __linux__ to have a private meaning inside
the kernel tree, and for us to do unifdef processing based on that.

Since this simplifies the change (and hence reduces the risk of
collateral damage), and since a few arches already do the same thing,
I'm now not strongly opposed to putting -D__linux__ in arch/arm/Makefile.

The I objection I had to that (building a non-Linux OS with a Linux
targeted compiler might require -U__linux__) is contrived, and I have
no real example for it.

Have I understood you correctly?

As you point out, some exportable userspace headers still need to be
cleaned up to use __linux__ in the appropriate way, and
scripts/headers_install.pl would need to be updated accordingly.
Since headers_install.pl is global, would that mean that -D__linux__
should also be global, in the top-level Makefile?

---Dave

[Linus Walleij (triad)]

I have this old AP from Linus (Torvalds) to make the kernel build with arm-none-*
baremetal compilers, currently there are more than a few obstacles to this:
https://lkml.org/lkml/2011/10/28/76

In this case:

-#ifdef linux
+#if defined(linux) || defined(__KERNEL__)
 #ifdef __KERNEL__
...

Which was tentatively ACK:ed by Torvalds.