Inconsistent "#ifdef __KERNEL__" on different architectures

Inconsistent "#ifdef __KERNEL__" on different architectures

[Adrian Bunk]

Hi,

while looking for the reason of a build failure of the ALSA libraries on
ARM [1] I discovered the following strange thing:

On some architectures a function is inside an "#ifdef __KERNEL__" in the
header file and on others not. Is there a reason for this or is this
inconsistency simply a bug?

In this case the following functions are affected (in 2.4.5):

atomic_read, atomic_inc and atomic_dec in include/asm-*/atomic.h

"#ifdef __KERNEL__" only on arm, mips, mips64 and sparc (but not on
                                                         sparc64)


rmb and wmb in include/asm-*/system.h

"#ifdef __KERNEL__" only on arm and sparc (but not on sparc64)

not defined on parisc although used to define smp_rmb on SMP systems:
<--  snip  -->
#ifdef CONFIG_SMP
#define smp_mb()        mb()
#define smp_rmb()       rmb()
#define smp_wmb()       wmb()
#else
<--  snip  -->


cu
Adrian

[1] http://bugs.debian.org/97988


-- 
A "No" uttered from deepest conviction is better and greater than a
"Yes" merely uttered to please, or what is worse, to avoid trouble.
                -- Mahatma Ghandi

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Alan Cox]

> On some architectures a function is inside an "#ifdef __KERNEL__" in the
> header file and on others not. Is there a reason for this or is this
> inconsistency simply a bug?

Its probably a bug - primarily it depends if the function is useful when
exported to non kernel code

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Abramo Bagnara]

Adrian Bunk wrote:
> 
> Hi,
> 
> while looking for the reason of a build failure of the ALSA libraries on
> ARM [1] I discovered the following strange thing:
> 
> On some architectures a function is inside an "#ifdef __KERNEL__" in the
> header file and on others not. Is there a reason for this or is this
> inconsistency simply a bug?

This is a bug on some architectures, I've personally fixed this on PPC
sending a patch to Cort Dougan. It has been included in 2.4.5.

I'd like you send a patch to maintainers (or perhaps to Alan).

-- 
Abramo Bagnara                       mailto:abramo@alsa-project.org

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project               http://www.alsa-project.org
It sounds good!

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Adrian Bunk]

On Sun, 27 May 2001, Abramo Bagnara wrote:

> Adrian Bunk wrote:
> >
> > while looking for the reason of a build failure of the ALSA libraries on
> > ARM [1] I discovered the following strange thing:
> >
> > On some architectures a function is inside an "#ifdef __KERNEL__" in the
> > header file and on others not. Is there a reason for this or is this
> > inconsistency simply a bug?
>
> This is a bug on some architectures, I've personally fixed this on PPC
> sending a patch to Cort Dougan. It has been included in 2.4.5.
>
> I'd like you send a patch to maintainers (or perhaps to Alan).

I do also explicitely send this mail to the people that seem to be
responsible for the pieces of code I touch.

I'm not sure whether the compete removal of the "#ifdef __KERNEL__"'s is
too rude but there are already other architectures that don't have it in
their architecture specific versions of these files.

A patch for the problems I mentioned (except for parisc) would be:

--- include/asm-arm/atomic.h.old	Sun May 27 22:30:58 2001
+++ include/asm-arm/atomic.h	Sun May 27 22:58:20 2001
@@ -12,6 +12,7 @@
  *   13-04-1997	RMK	Made functions atomic!
  *   07-12-1997	RMK	Upgraded for v2.1.
  *   26-08-1998	PJB	Added #ifdef __KERNEL__
+ *   27-05-2001	APB     Removed #ifdef __KERNEL__
  */
 #ifndef __ASM_ARM_ATOMIC_H
 #define __ASM_ARM_ATOMIC_H
@@ -30,7 +31,6 @@

 #define ATOMIC_INIT(i)	{ (i) }

-#ifdef __KERNEL__
 #include <asm/proc/system.h>

 #define atomic_read(v)	((v)->counter)
@@ -107,5 +107,4 @@
 	__restore_flags(flags);
 }

-#endif
 #endif
--- include/asm-arm/system.h.old	Sun May 27 22:31:47 2001
+++ include/asm-arm/system.h	Sun May 27 22:31:56 2001
@@ -1,8 +1,6 @@
 #ifndef __ASM_ARM_SYSTEM_H
 #define __ASM_ARM_SYSTEM_H

-#ifdef __KERNEL__
-
 #include <linux/config.h>
 #include <linux/kernel.h>

@@ -84,7 +82,5 @@
 #define save_flags_cli(x)	__save_flags_cli(x)

 #endif /* CONFIG_SMP */
-
-#endif /* __KERNEL__ */

 #endif
--- include/asm-sparc/atomic.h.old	Sun May 27 22:32:29 2001
+++ include/asm-sparc/atomic.h	Sun May 27 22:32:48 2001
@@ -11,7 +11,6 @@

 typedef struct { volatile int counter; } atomic_t;

-#ifdef __KERNEL__
 #ifndef CONFIG_SMP

 #define ATOMIC_INIT(i)  { (i) }
@@ -99,7 +98,5 @@
 #define atomic_dec(v) ((void)__atomic_sub(1, (v)))

 #define atomic_add_negative(i, v) (__atomic_add((i), (v)) < 0)
-
-#endif /* !(__KERNEL__) */

 #endif /* !(__ARCH_SPARC_ATOMIC__) */
--- include/asm-sparc/system.h.old	Sun May 27 22:32:50 2001
+++ include/asm-sparc/system.h	Sun May 27 22:33:52 2001
@@ -33,8 +33,6 @@
   ap1000      = 0x07, /* almost a sun4m */
 };

-/* Really, userland should not be looking at any of this... */
-#ifdef __KERNEL__

 extern enum sparc_cpu sparc_cpu_model;

@@ -335,8 +333,6 @@
 }

 extern void die_if_kernel(char *str, struct pt_regs *regs) __attribute__ ((noreturn));
-
-#endif /* __KERNEL__ */

 #endif /* __ASSEMBLY__ */

--- include/asm-mips/atomic.h.old	Sun May 27 22:38:35 2001
+++ include/asm-mips/atomic.h	Sun May 27 22:38:48 2001
@@ -24,7 +24,6 @@
 typedef struct { int counter; } atomic_t;
 #endif

-#ifdef __KERNEL__
 #define ATOMIC_INIT(i)    { (i) }

 #define atomic_read(v)	((v)->counter)
@@ -199,6 +198,5 @@

 #define atomic_inc(v) atomic_add(1,(v))
 #define atomic_dec(v) atomic_sub(1,(v))
-#endif /* defined(__KERNEL__) */

 #endif /* __ASM_MIPS_ATOMIC_H */
--- include/asm-mips64/atomic.h.old	Sun May 27 22:38:53 2001
+++ include/asm-mips64/atomic.h	Sun May 27 22:39:02 2001
@@ -18,7 +18,6 @@

 typedef struct { volatile int counter; } atomic_t;

-#ifdef __KERNEL__
 #define ATOMIC_INIT(i)    { (i) }

 #define atomic_read(v)	((v)->counter)
@@ -99,6 +98,5 @@

 #define atomic_inc(v) atomic_add(1,(v))
 #define atomic_dec(v) atomic_sub(1,(v))
-#endif /* defined(__KERNEL__) */

 #endif /* _ASM_ATOMIC_H */


cu
Adrian

-- 
A "No" uttered from deepest conviction is better and greater than a
"Yes" merely uttered to please, or what is worse, to avoid trouble.
                -- Mahatma Ghandi

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Russell King - ARM Linux]

On Sun, May 27, 2001 at 11:07:38PM +0200, Adrian Bunk wrote:
> I do also explicitely send this mail to the people that seem to be
> responsible for the pieces of code I touch.
> 
> I'm not sure whether the compete removal of the "#ifdef __KERNEL__"'s is
> too rude but there are already other architectures that don't have it in
> their architecture specific versions of these files.

You cannot use the kernel atomic/interrupt functions from userspace
on ARM.  You cannot disable interrupts in userspace, and therefore the
kernel atomic functions do not work as you expect them to.

If it is to do with code to be included into the kernel, then why aren't
you defining __KERNEL__ ?

Therefore this change (as far as ARM goes) makes zero sense.

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Philip Blundell]

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

>--- include/asm-arm/atomic.h.old	Sun May 27 22:30:58 2001
>+++ include/asm-arm/atomic.h	Sun May 27 22:58:20 2001
>@@ -12,6 +12,7 @@
>  *   13-04-1997	RMK	Made functions atomic!
>  *   07-12-1997	RMK	Upgraded for v2.1.
>  *   26-08-1998	PJB	Added #ifdef __KERNEL__
>+ *   27-05-2001	APB     Removed #ifdef __KERNEL__
>  */
> #ifndef __ASM_ARM_ATOMIC_H
> #define __ASM_ARM_ATOMIC_H
>@@ -30,7 +31,6 @@

This is no good.  The ARM kernel just doesn't provide any atomic primitives 
that will work in user space.  If you want atomicity you have to use 
libpthread.

p.



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

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Ralf Baechle]

On Sun, May 27, 2001 at 11:10:00PM +0100, Philip Blundell wrote:

> >--- include/asm-arm/atomic.h.old	Sun May 27 22:30:58 2001
> >+++ include/asm-arm/atomic.h	Sun May 27 22:58:20 2001
> >@@ -12,6 +12,7 @@
> >  *   13-04-1997	RMK	Made functions atomic!
> >  *   07-12-1997	RMK	Upgraded for v2.1.
> >  *   26-08-1998	PJB	Added #ifdef __KERNEL__
> >+ *   27-05-2001	APB     Removed #ifdef __KERNEL__
> >  */
> > #ifndef __ASM_ARM_ATOMIC_H
> > #define __ASM_ARM_ATOMIC_H
> >@@ -30,7 +31,6 @@
> 
> This is no good.  The ARM kernel just doesn't provide any atomic primitives 
> that will work in user space.  If you want atomicity you have to use 
> libpthread.

Similar on some MIPS processors where the kernel has to implement atomic
operations because there is no practical possibility to implement them
in userspace.

  Ralf

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Adrian Bunk]

On Wed, 30 May 2001, Ralf Baechle wrote:

> > This is no good.  The ARM kernel just doesn't provide any atomic primitives
> > that will work in user space.  If you want atomicity you have to use
> > libpthread.
>
> Similar on some MIPS processors where the kernel has to implement atomic
> operations because there is no practical possibility to implement them
> in userspace.

Thanks for the answer, this means:
It is specific to the arm, mips, mips64 and sparc ports that atomic_read,
atomic_inc and atomic_dec won't work in user space and they do work on all
other architectures.

(my main concern wasn't whether the "#ifdef __KERNEL__" is correct or not
but I was wondering whether there's a reason why it's different on
different architectures)

>   Ralf

cu
Adrian

-- 
A "No" uttered from deepest conviction is better and greater than a
"Yes" merely uttered to please, or what is worse, to avoid trouble.
                -- Mahatma Ghandi

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Paul Mackerras]

Adrian Bunk writes:

> (my main concern wasn't whether the "#ifdef __KERNEL__" is correct or not
> but I was wondering whether there's a reason why it's different on
> different architectures)

The only valid reason for userspace programs to be including kernel
headers is to get definitions that are part of the kernel API.  (And
in fact others here will go further and assert that there are *no*
valid reasons for userspace programs to include kernel headers.)

If you want some atomic functions or whatever for your userspace
program and the ones in the kernel look like they would be useful,
then take a copy of the relevant kernel code if you like, but don't
include the kernel headers directly.  If you do, you will get bitten
at some point in the future when we decide to change some internal
implementation detail in the kernel, and your program suddenly won't
compile any more.

This is why I added #ifdef __KERNEL__ around most of the contents
of include/asm-ppc/*.h.  It was done deliberately to flush out those
programs which are depending on kernel headers when they shouldn't.

Paul.

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Adrian Bunk]

On Tue, 5 Jun 2001, Paul Mackerras wrote:

>...
> This is why I added #ifdef __KERNEL__ around most of the contents
> of include/asm-ppc/*.h.  It was done deliberately to flush out those
> programs which are depending on kernel headers when they shouldn't.

Whatever the right policy is, the main concern in my initial mail was the
_consistency_ of the kernel headers between different architectures.
So when you want to flush out these programs I see no reason to
inconsistetly change it only on one architecture.


> Paul.

cu
Adrian

-- 
A "No" uttered from deepest conviction is better and greater than a
"Yes" merely uttered to please, or what is worse, to avoid trouble.
                -- Mahatma Ghandi

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Paul Mackerras]

Adrian Bunk writes:

> Whatever the right policy is, the main concern in my initial mail was the
> _consistency_ of the kernel headers between different architectures.
> So when you want to flush out these programs I see no reason to
> inconsistetly change it only on one architecture.

Different architectures are maintained by different people who have
different perspectives on things.  The only thing you have any right
to expect any consistency in is the kernel API, and even there things
like error numbers etc. differ between architectures.

If you want consistency, you would either have to persuade Linus to
issue an edict or else persuade every single architecture maintainer
to do things the same way.  But if the motivation is to make it easier
for user-level programs to use things which are not intended to be
exported to userspace, then all you will achieve is that we will make
sure that you can't use those things from userspace.  And this
definitely includes things like atomics, bitops, memory barriers etc.
Take a copy by all means but don't rely on the kernel definitions for
your userspace programs.

It is the policy for all architectures that kernel headers should not
be used in userspace programs.  The "inconsistency" that you are
complaining about is only a difference in the extent to which
this policy is enforced.

Paul.

Re: Inconsistent "#ifdef __KERNEL__" on different architectures

[Albert D. Cahalan]

Paul Mackerras writes:

> The only valid reason for userspace programs to be including kernel
> headers is to get definitions that are part of the kernel API.  (And
> in fact others here will go further and assert that there are *no*
> valid reasons for userspace programs to include kernel headers.)
>
> If you want some atomic functions or whatever for your userspace
> program and the ones in the kernel look like they would be useful,
> then take a copy of the relevant kernel code if you like, but don't
> include the kernel headers directly.

Sure. That copy belongs in /usr/include/asm for all programs
to use, and it should match the libc that will be linked against.
(note: "copy", not a symlink)

Red Hat 7 gets this right:

$ ls -ldog /usr/include/asm /usr/include/linux            
drwxr-xr-x    2 root         2048 Sep 28  2000 /usr/include/asm
drwxr-xr-x   10 root        10240 Sep 28  2000 /usr/include/linux

Debian's "unstable" is correct too:

$ ls -ldog /usr/include/asm /usr/include/linux
drwxr-xr-x    2 root         6144 Mar 12 15:57 /usr/include/asm
drwxr-xr-x   10 root        23552 Mar 12 15:57 /usr/include/linux

> This is why I added #ifdef __KERNEL__ around most of the contents
> of include/asm-ppc/*.h.  It was done deliberately to flush out those
> programs which are depending on kernel headers when they shouldn't.

What, is </usr/src/linux/asm/foo.h> being used? I doubt it.

If /usr/include/asm is a link into /usr/src/linux, then you
have a problem with your Linux distribution. Don't blame the
apps for this problem.

Adding "#ifdef __KERNEL__" causes extra busywork for someone
trying to adapt kernel headers for userspace use. At least do
something easy to rip out. Three lines, all together at the top:

#ifndef __KERNEL__
#error Raw kernel headers may not be compatible with user code.
#endif