Confusion in usr/include/asm-generic/fcntl.h

Confusion in usr/include/asm-generic/fcntl.h

[Jaswinder Singh Rajput]

usr/include/asm-generic/fcntl.h is giving 2 'make headers_check' warnings:
 usr/include/asm-generic/fcntl.h:127: leaks CONFIG_64BIT to userspace where it is not valid
 usr/include/asm-generic/fcntl.h:149: leaks CONFIG_64BIT to userspace where it is not valid

usr/include/asm-generic/fcntl.h:
--
#ifndef CONFIG_64BIT

#ifndef F_GETLK64
#define F_GETLK64       12      /*  using 'struct flock64' */
#define F_SETLK64       13
#define F_SETLKW64      14
#endif

#ifndef HAVE_ARCH_STRUCT_FLOCK64
#ifndef __ARCH_FLOCK64_PAD
#define __ARCH_FLOCK64_PAD
#endif

struct flock64 {
        short  l_type;
        short  l_whence;
        loff_t l_start;
        loff_t l_len;
        pid_t  l_pid;
        __ARCH_FLOCK64_PAD
};
#endif
#endif /* !CONFIG_64BIT */
--

#ifndef CONFIG_64BIT will always be true for userspace. So what is the use of #ifndef CONFIG_64BIT ?

Thanks,
--
JSR

Re: Confusion in usr/include/asm-generic/fcntl.h

[David Miller]

From: Jaswinder Singh Rajput <jaswinder@kernel.org>
Date: Wed, 21 Jan 2009 05:34:17 +0530

> usr/include/asm-generic/fcntl.h is giving 2 'make headers_check' warnings:
>  usr/include/asm-generic/fcntl.h:127: leaks CONFIG_64BIT to userspace where it is not valid
>  usr/include/asm-generic/fcntl.h:149: leaks CONFIG_64BIT to userspace where it is not valid
> 
> usr/include/asm-generic/fcntl.h:
...
> #ifndef CONFIG_64BIT will always be true for userspace. So what is the use of #ifndef CONFIG_64BIT ?

Good catch.

This file needs to test for 64-bit'ness using some non-CONFIG_*
test.  And the standard built-in CPP macros which can be used
to check for that are different on every platform.

Re: Confusion in usr/include/asm-generic/fcntl.h

[Arnd Bergmann]

On Wednesday 21 January 2009, David Miller wrote:
> From: Jaswinder Singh Rajput <jaswinder@kernel.org>
> Date: Wed, 21 Jan 2009 05:34:17 +0530
> 
> > usr/include/asm-generic/fcntl.h is giving 2 'make headers_check' warnings:
> >  usr/include/asm-generic/fcntl.h:127: leaks CONFIG_64BIT to userspace where it is not valid
> >  usr/include/asm-generic/fcntl.h:149: leaks CONFIG_64BIT to userspace where it is not valid
> > 
> > usr/include/asm-generic/fcntl.h:
> ...
> > #ifndef CONFIG_64BIT will always be true for userspace. So what is the use of #ifndef CONFIG_64BIT ?
> 
> Good catch.
> 
> This file needs to test for 64-bit'ness using some non-CONFIG_*
> test.  And the standard built-in CPP macros which can be used
> to check for that are different on every platform.

I think we should simply define a macro for use in the kernel, e.g. in
<asm/types.h>. There already is a BITS_PER_LONG macro in there, maybe
we can add an exported __BITS_PER_LONG there that checks for the right
macro on each architecture.

On parisc, there is a major confusion in this area, at some point, all
checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
not understood what the problem with __LP64__ was, the check for
CONFIG_64BIT on parisc user space looks very wrong.

	Arnd <><

Re: Confusion in usr/include/asm-generic/fcntl.h

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 21 Jan 2009 01:24:25 +0100

> I think we should simply define a macro for use in the kernel, e.g. in
> <asm/types.h>. There already is a BITS_PER_LONG macro in there, maybe
> we can add an exported __BITS_PER_LONG there that checks for the right
> macro on each architecture.

That might work.

> On parisc, there is a major confusion in this area, at some point, all
> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
> not understood what the problem with __LP64__ was, the check for
> CONFIG_64BIT on parisc user space looks very wrong.

Yep.

Re: Confusion in usr/include/asm-generic/fcntl.h

[Helge Deller]

From: Arnd Bergmann <arnd@arndb.de>
>> On parisc, there is a major confusion in this area, at some point, all
>> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
>> not understood what the problem with __LP64__ was, the check for
>> CONFIG_64BIT on parisc user space looks very wrong.

I think the parisc mess is my fault. I once replaced the __LP64__ by 
CONFIG_64BIT and forgot that some files are exported to userspace.
I'll clean that up and send patches.

Thanks for catching this!

Helge

Re: Confusion in usr/include/asm-generic/fcntl.h

[Arnd Bergmann]

On Wednesday 21 January 2009, Helge Deller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> >> On parisc, there is a major confusion in this area, at some point, all
> >> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
> >> not understood what the problem with __LP64__ was, the check for
> >> CONFIG_64BIT on parisc user space looks very wrong.
> 
> I think the parisc mess is my fault. I once replaced the __LP64__ by 
> CONFIG_64BIT and forgot that some files are exported to userspace.
> I'll clean that up and send patches.

I have a patch set that introduces a lot more asm-generic headers where
I also need a generic way to check for this. The approach I chose
here was to check "#if __BITS_PER_LONG == 64" for anything that is
exported to user space. Maybe you can #define this in asm/types.h
and use this check in the parisc headers as well.

	Arnd <><

Re: Confusion in usr/include/asm-generic/fcntl.h

[Sam Ravnborg]

On Wed, Jan 21, 2009 at 09:24:22AM +0100, Arnd Bergmann wrote:
> On Wednesday 21 January 2009, Helge Deller wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > >> On parisc, there is a major confusion in this area, at some point, all
> > >> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
> > >> not understood what the problem with __LP64__ was, the check for
> > >> CONFIG_64BIT on parisc user space looks very wrong.
> > 
> > I think the parisc mess is my fault. I once replaced the __LP64__ by 
> > CONFIG_64BIT and forgot that some files are exported to userspace.
> > I'll clean that up and send patches.
> 
> I have a patch set that introduces a lot more asm-generic headers where
> I also need a generic way to check for this. The approach I chose
> here was to check "#if __BITS_PER_LONG == 64" for anything that is
> exported to user space. Maybe you can #define this in asm/types.h
> and use this check in the parisc headers as well.

Could we add a new symbol for this?
We know we are going to use this in several places so a simpler variant
would be more readable.

Something like:

#ifdef __64BIT
...
#endif


When we define __64BIT we would use the  __BITS_PER_LONG == 64 check.

	Sam

Re: Confusion in usr/include/asm-generic/fcntl.h

[Arnd Bergmann]

On Wednesday 21 January 2009, Sam Ravnborg wrote:
> Could we add a new symbol for this?
> We know we are going to use this in several places so a simpler variant
> would be more readable.
> 
> Something like:
> 
> #ifdef __64BIT
> ...
> #endif
> 
> When we define __64BIT we would use the  __BITS_PER_LONG == 64 check.

I would prefer using the __BITS_PER_LONG == 64 check directly, because
it gives you a warning when __BITS_PER_LONG is undefined, whereas the
#ifdef check gets easily fooled by include order problems. Note that
this is not a problem in the kernel for CONFIG_* symbols which are
always defined before the first #include.

	Arnd <><

Re: Confusion in usr/include/asm-generic/fcntl.h

[Kyle McMartin]

On Wed, Jan 21, 2009 at 01:13:16PM +0100, Arnd Bergmann wrote:
> On Wednesday 21 January 2009, Sam Ravnborg wrote:
> > Could we add a new symbol for this?
> > We know we are going to use this in several places so a simpler variant
> > would be more readable.
> > 
> > Something like:
> > 
> > #ifdef __64BIT
> > ...
> > #endif
> > 
> > When we define __64BIT we would use the  __BITS_PER_LONG == 64 check.
> 
> I would prefer using the __BITS_PER_LONG == 64 check directly, because
> it gives you a warning when __BITS_PER_LONG is undefined, whereas the
> #ifdef check gets easily fooled by include order problems. Note that
> this is not a problem in the kernel for CONFIG_* symbols which are
> always defined before the first #include.
> 

This is cool with me Arnd. Rock on. :)

cheers, Kyle

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

Arnd Bergmann wrote:
> 
> I would prefer using the __BITS_PER_LONG == 64 check directly, because
> it gives you a warning when __BITS_PER_LONG is undefined, whereas the
> #ifdef check gets easily fooled by include order problems. Note that
> this is not a problem in the kernel for CONFIG_* symbols which are
> always defined before the first #include.
> 

I fully agree with this.  It actually *is* a problem for CONFIG_*
symbols too, since people typo them all the time.

	-hpa

Re: Confusion in usr/include/asm-generic/fcntl.h

[Sam Ravnborg]

On Wed, Jan 21, 2009 at 01:13:16PM +0100, Arnd Bergmann wrote:
> On Wednesday 21 January 2009, Sam Ravnborg wrote:
> > Could we add a new symbol for this?
> > We know we are going to use this in several places so a simpler variant
> > would be more readable.
> > 
> > Something like:
> > 
> > #ifdef __64BIT
> > ...
> > #endif
> > 
> > When we define __64BIT we would use the  __BITS_PER_LONG == 64 check.
> 
> I would prefer using the __BITS_PER_LONG == 64 check directly, because
> it gives you a warning when __BITS_PER_LONG is undefined, whereas the
> #ifdef check gets easily fooled by include order problems. Note that
> this is not a problem in the kernel for CONFIG_* symbols which are
> always defined before the first #include.

It gives the warning only if you add -Wundef which IIRC is not default
with -Wall. And using the "__BITS_PER_LONG == 64" the risk of gitting
the expression wrong is much higher than the simpler variant where
you only write:

    __64BIT

But I have no strong feelings for it.

	Sam

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

Sam Ravnborg wrote:
> 
> It gives the warning only if you add -Wundef which IIRC is not default
> with -Wall. And using the "__BITS_PER_LONG == 64" the risk of gitting
> the expression wrong is much higher than the simpler variant where
> you only write:
> 
>     __64BIT
> 
> But I have no strong feelings for it.
> 

It's not the default, but it would be nice if we could start using it.

	-hpa

Re: Confusion in usr/include/asm-generic/fcntl.h

[Helge Deller]

Arnd Bergmann wrote:
> On Wednesday 21 January 2009, Helge Deller wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>>> On parisc, there is a major confusion in this area, at some point, all
>>>> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
>>>> not understood what the problem with __LP64__ was, the check for
>>>> CONFIG_64BIT on parisc user space looks very wrong.
>> I think the parisc mess is my fault. I once replaced the __LP64__ by 
>> CONFIG_64BIT and forgot that some files are exported to userspace.
>> I'll clean that up and send patches.
> 
> I have a patch set that introduces a lot more asm-generic headers where
> I also need a generic way to check for this. The approach I chose
> here was to check "#if __BITS_PER_LONG == 64" for anything that is
> exported to user space. Maybe you can #define this in asm/types.h
> and use this check in the parisc headers as well.

As per your suggestion, the patch below fixes the problem that the 
CONFIG_64BIT kernel config option is exported to userspace on parisc by mistake.
Tested on 32- and 64-bit parisc kernel builds and with "make headers_check".

arch/parisc/include/asm/:
 assembly.h    |   20 +++++++++++---------
 msgbuf.h      |    8 +++++---
 pdc.h         |   12 +++++-------
 posix_types.h |    4 +++-
 sembuf.h      |    4 ++--
 shmbuf.h      |   13 +++++++------
 signal.h      |    2 +-
 swab.h        |    2 +-
 types.h       |   10 ++++++++++
 9 files changed, 45 insertions(+), 30 deletions(-)

--------------------
parisc: do not export kernel config options to userspace in asm/ header files

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/include/asm/assembly.h b/arch/parisc/include/asm/assembly.h
index ab7cc37..b950843 100644
--- a/arch/parisc/include/asm/assembly.h
+++ b/arch/parisc/include/asm/assembly.h
@@ -21,9 +21,11 @@
 #ifndef _PARISC_ASSEMBLY_H
 #define _PARISC_ASSEMBLY_H
 
+#include <asm/types.h>
+
 #define CALLEE_FLOAT_FRAME_SIZE	80
 
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 #define LDREG	ldd
 #define STREG	std
 #define LDREGX  ldd,s
@@ -37,7 +39,7 @@
 #define FRAME_SIZE	128
 #define CALLEE_REG_FRAME_SIZE	144
 #define ASM_ULONG_INSN	.dword
-#else	/* CONFIG_64BIT */
+#else	/* __BITS_PER_LONG == 64 */
 #define LDREG	ldw
 #define STREG	stw
 #define LDREGX  ldwx,s
@@ -58,7 +60,7 @@
 #ifdef CONFIG_PA20
 #define LDCW		ldcw,co
 #define BL		b,l
-# ifdef CONFIG_64BIT
+# if __BITS_PER_LONG == 64
 #  define LEVEL		2.0w
 # else
 #  define LEVEL		2.0
@@ -71,7 +73,7 @@
 
 #ifdef __ASSEMBLY__
 
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 /* the 64-bit pa gnu assembler unfortunately defaults to .level 1.1 or 2.0 so
  * work around that for now... */
 	.level 2.0w
@@ -162,7 +164,7 @@
 	.endm
 
 	.macro loadgp
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 	ldil		L%__gp, %r27
 	ldo		R%__gp(%r27), %r27
 #else
@@ -340,7 +342,7 @@
 	fldd,mb	-8(%r30),   %fr12
 	.endm
 
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 	.macro	callee_save
 	std,ma	  %r3,	 CALLEE_REG_FRAME_SIZE(%r30)
 	mfctl	  %cr27, %r3
@@ -383,7 +385,7 @@
 	ldd,mb	-CALLEE_REG_FRAME_SIZE(%r30),    %r3
 	.endm
 
-#else /* ! CONFIG_64BIT */
+#else /* ! __BITS_PER_LONG == 64 */
 
 	.macro	callee_save
 	stw,ma	 %r3,	CALLEE_REG_FRAME_SIZE(%r30)
@@ -426,7 +428,7 @@
 	mtctl	%r3, %cr27
 	ldw,mb	-CALLEE_REG_FRAME_SIZE(%r30),   %r3
 	.endm
-#endif /* ! CONFIG_64BIT */
+#endif /* ! __BITS_PER_LONG == 64 */
 
 	.macro	save_specials	regs
 
@@ -447,7 +449,7 @@
 	mtctl	 %r0,	%cr18
 	SAVE_CR  (%cr18, PT_IAOQ1(\regs))
 
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 	/* cr11 (sar) is a funny one.  5 bits on PA1.1 and 6 bit on PA2.0
 	 * For PA2.0 mtsar or mtctl always write 6 bits, but mfctl only
 	 * reads 5 bits.  Use mfctl,w to read all six bits.  Otherwise
diff --git a/arch/parisc/include/asm/msgbuf.h b/arch/parisc/include/asm/msgbuf.h
index fe88f26..a4520f5 100644
--- a/arch/parisc/include/asm/msgbuf.h
+++ b/arch/parisc/include/asm/msgbuf.h
@@ -1,6 +1,8 @@
 #ifndef _PARISC_MSGBUF_H
 #define _PARISC_MSGBUF_H
 
+#include <linux/types.h>
+
 /* 
  * The msqid64_ds structure for parisc architecture, copied from sparc.
  * Note extra padding because this structure is passed back and forth
@@ -13,15 +15,15 @@
 
 struct msqid64_ds {
 	struct ipc64_perm msg_perm;
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int   __pad1;
 #endif
 	__kernel_time_t msg_stime;	/* last msgsnd time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int   __pad2;
 #endif
 	__kernel_time_t msg_rtime;	/* last msgrcv time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int   __pad3;
 #endif
 	__kernel_time_t msg_ctime;	/* last change time */
diff --git a/arch/parisc/include/asm/pdc.h b/arch/parisc/include/asm/pdc.h
index c584b00..f118764 100644
--- a/arch/parisc/include/asm/pdc.h
+++ b/arch/parisc/include/asm/pdc.h
@@ -336,10 +336,11 @@
 #define NUM_PDC_RESULT	32
 
 #if !defined(__ASSEMBLY__)
-#ifdef __KERNEL__
 
 #include <linux/types.h>
 
+#ifdef __KERNEL__
+
 extern int pdc_type;
 
 /* Values for pdc_type */
@@ -374,7 +375,7 @@ struct pdc_model {		/* for PDC_MODEL */
 
 struct pdc_cache_cf {		/* for PDC_CACHE  (I/D-caches) */
     unsigned long
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 		cc_padW:32,
 #endif
 		cc_alias: 4,	/* alias boundaries for virtual addresses   */
@@ -390,7 +391,7 @@ struct pdc_cache_cf {		/* for PDC_CACHE  (I/D-caches) */
 
 struct pdc_tlb_cf {		/* for PDC_CACHE (I/D-TLB's) */
     unsigned long tc_pad0:12,	/* reserved */
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 		tc_padW:32,
 #endif
 		tc_sh	: 2,	/* 0 = separate I/D-TLB, else shared I/D-TLB */
@@ -478,7 +479,6 @@ struct pdc_btlb_info {	/* PDC_BLOCK_TLB, return of PDC_BTLB_INFO */
 
 #endif /* !CONFIG_PA20 */
 
-#ifdef CONFIG_64BIT
 struct pdc_memory_table_raddr { /* PDC_MEM/PDC_MEM_TABLE (return info) */
 	unsigned long entries_returned;
 	unsigned long entries_total;
@@ -489,7 +489,6 @@ struct pdc_memory_table {       /* PDC_MEM/PDC_MEM_TABLE (arguments) */
 	unsigned int  pages;
 	unsigned int  reserved;
 };
-#endif /* CONFIG_64BIT */
 
 struct pdc_system_map_mod_info { /* PDC_SYSTEM_MAP/FIND_MODULE */
 	unsigned long mod_addr;
@@ -636,10 +635,9 @@ int pdc_get_initiator(struct hardware_path *, struct pdc_initiator *);
 int pdc_tod_read(struct pdc_tod *tod);
 int pdc_tod_set(unsigned long sec, unsigned long usec);
 
-#ifdef CONFIG_64BIT
+/* pdc_mem_mem_table - only available on 64bit */
 int pdc_mem_mem_table(struct pdc_memory_table_raddr *r_addr,
 		struct pdc_memory_table *tbl, unsigned long entries);
-#endif
 
 void set_firmware_width(void);
 void set_firmware_width_unlocked(void);
diff --git a/arch/parisc/include/asm/posix_types.h b/arch/parisc/include/asm/posix_types.h
index 00da29a..6946cdc 100644
--- a/arch/parisc/include/asm/posix_types.h
+++ b/arch/parisc/include/asm/posix_types.h
@@ -1,6 +1,8 @@
 #ifndef __ARCH_PARISC_POSIX_TYPES_H
 #define __ARCH_PARISC_POSIX_TYPES_H
 
+#include <asm/types.h>
+
 /*
  * This file is generally used by user-level software, so you need to
  * be a little careful about namespace pollution etc.  Also, we cannot
@@ -20,7 +22,7 @@ typedef int			__kernel_timer_t;
 typedef int			__kernel_clockid_t;
 typedef int			__kernel_daddr_t;
 /* Note these change from narrow to wide kernels */
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 typedef unsigned long		__kernel_size_t;
 typedef long			__kernel_ssize_t;
 typedef long			__kernel_ptrdiff_t;
diff --git a/arch/parisc/include/asm/sembuf.h b/arch/parisc/include/asm/sembuf.h
index 1e59ffd..d27e904 100644
--- a/arch/parisc/include/asm/sembuf.h
+++ b/arch/parisc/include/asm/sembuf.h
@@ -13,11 +13,11 @@
 
 struct semid64_ds {
 	struct ipc64_perm sem_perm;		/* permissions .. see ipc.h */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int	__pad1;
 #endif
 	__kernel_time_t	sem_otime;		/* last semop time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int	__pad2;
 #endif
 	__kernel_time_t	sem_ctime;		/* last change time */
diff --git a/arch/parisc/include/asm/shmbuf.h b/arch/parisc/include/asm/shmbuf.h
index 0a3eada..d133bf4 100644
--- a/arch/parisc/include/asm/shmbuf.h
+++ b/arch/parisc/include/asm/shmbuf.h
@@ -1,6 +1,8 @@
 #ifndef _PARISC_SHMBUF_H
 #define _PARISC_SHMBUF_H
 
+#include <linux/types.h>
+
 /* 
  * The shmid64_ds structure for parisc architecture.
  * Note extra padding because this structure is passed back and forth
@@ -13,19 +15,19 @@
 
 struct shmid64_ds {
 	struct ipc64_perm	shm_perm;	/* operation perms */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int		__pad1;
 #endif
 	__kernel_time_t		shm_atime;	/* last attach time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int		__pad2;
 #endif
 	__kernel_time_t		shm_dtime;	/* last detach time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int		__pad3;
 #endif
 	__kernel_time_t		shm_ctime;	/* last change time */
-#ifndef CONFIG_64BIT
+#if __BITS_PER_LONG == 32
 	unsigned int		__pad4;
 #endif
 	size_t			shm_segsz;	/* size of segment (bytes) */
@@ -36,13 +38,12 @@ struct shmid64_ds {
 	unsigned int		__unused2;
 };
 
-#ifdef CONFIG_64BIT
+
 /* The 'unsigned int' (formerly 'unsigned long') data types below will
  * ensure that a 32-bit app calling shmctl(*,IPC_INFO,*) will work on
  * a wide kernel, but if some of these values are meant to contain pointers
  * they may need to be 'long long' instead. -PB XXX FIXME
  */
-#endif
 struct shminfo64 {
 	unsigned int	shmmax;
 	unsigned int	shmmin;
diff --git a/arch/parisc/include/asm/signal.h b/arch/parisc/include/asm/signal.h
index c203563..3de6265 100644
--- a/arch/parisc/include/asm/signal.h
+++ b/arch/parisc/include/asm/signal.h
@@ -105,7 +105,7 @@
 struct siginfo;
 
 /* Type of a signal handler.  */
-#ifdef CONFIG_64BIT
+#if __BITS_PER_LONG == 64
 /* function pointers on 64-bit parisc are pointers to little structs and the
  * compiler doesn't support code which changes or tests the address of
  * the function in the little struct.  This is really ugly -PB
diff --git a/arch/parisc/include/asm/swab.h b/arch/parisc/include/asm/swab.h
index 3ff16c5..e78403b 100644
--- a/arch/parisc/include/asm/swab.h
+++ b/arch/parisc/include/asm/swab.h
@@ -1,7 +1,7 @@
 #ifndef _PARISC_SWAB_H
 #define _PARISC_SWAB_H
 
-#include <asm/types.h>
+#include <linux/types.h>
 #include <linux/compiler.h>
 
 #define __SWAB_64_THRU_32__
diff --git a/arch/parisc/include/asm/types.h b/arch/parisc/include/asm/types.h
index 7f5a39b..14bb5bd 100644
--- a/arch/parisc/include/asm/types.h
+++ b/arch/parisc/include/asm/types.h
@@ -9,6 +9,14 @@ typedef unsigned short umode_t;
 
 #endif /* __ASSEMBLY__ */
 
+#ifndef __KERNEL__
+#ifdef __LP64__
+# define __BITS_PER_LONG 64
+#else
+# define __BITS_PER_LONG 32
+#endif /* __LP64__ */
+#endif /* __KERNEL__ */
+
 /*
  * These aren't exported outside the kernel to avoid name space clashes
  */
@@ -22,6 +30,8 @@ typedef unsigned short umode_t;
 #define SHIFT_PER_LONG 5
 #endif
 
+#define __BITS_PER_LONG	BITS_PER_LONG
+
 #ifndef __ASSEMBLY__
 
 /* Dma addresses are 32-bits wide.  */

Re: Confusion in usr/include/asm-generic/fcntl.h

[Grant Grundler]

On Wed, Jan 21, 2009 at 01:24:25AM +0100, Arnd Bergmann wrote:
...
> > This file needs to test for 64-bit'ness using some non-CONFIG_*
> > test.  And the standard built-in CPP macros which can be used
> > to check for that are different on every platform.
> 
> I think we should simply define a macro for use in the kernel, e.g. in
> <asm/types.h>. There already is a BITS_PER_LONG macro in there, maybe
> we can add an exported __BITS_PER_LONG there that checks for the right
> macro on each architecture.

I'm pretty sure __LP64__ is the standard compiler defined macro
for 64-bit builds on most architectures.

> On parisc, there is a major confusion in this area, at some point, all
> checks for __LP64__ got replaced with CONFIG_64BIT there. While I have
> not understood what the problem with __LP64__ was,

IIRC, the problem with __LP64__ is it wasn't visible to the CONFIG
tool chains and dependencies. I believe that's fixed now and parisc
could deprecate use of CONFIG_64BIT if there is a strong preference
for __LP64__.

> the check for
> CONFIG_64BIT on parisc user space looks very wrong.

This shouldn't be exported to user space. Is that what you meant?
PA-RISC user space can currently only be 32-bit anyway.
So a 64-bit kernel will have to take that into consideration.

thanks,
grant

Re: Confusion in usr/include/asm-generic/fcntl.h

[John David Anglin]

> > CONFIG_64BIT on parisc user space looks very wrong.
> 
> This shouldn't be exported to user space. Is that what you meant?

This does get exported to user space as applications sometimes need
to know whether they are running under a 32 or 64-bit kernel.  See
config.guess.  However, it certainly would be wrong to use this
to make decisions about user space.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

John David Anglin wrote:
>>> CONFIG_64BIT on parisc user space looks very wrong.
>> This shouldn't be exported to user space. Is that what you meant?
> 
> This does get exported to user space as applications sometimes need
> to know whether they are running under a 32 or 64-bit kernel.  See
> config.guess.  However, it certainly would be wrong to use this
> to make decisions about user space.
> 

CONFIG_* macros are not (or at least should not) be exported to 
userspace!  Userspace should not make compile-time decisions based on 
kernel configuration -- a runtime property!

	-hpa

Re: Confusion in usr/include/asm-generic/fcntl.h

[John David Anglin]

> CONFIG_* macros are not (or at least should not) be exported to 
> userspace!  Userspace should not make compile-time decisions based on 
> kernel configuration -- a runtime property!

Unfortunately, this decision was made years ago...

dave@mx3210:~$ uname -m
parisc64
dave@hiauly6:~/gnu/gcc-4.4/gcc$ uname -m
parisc

The only significant difference between the above two machines is that
mx3210 is running a 64-bit kernel and hiauly6 is running a 32-bit kernel.
Both are PA 2.0 machines.  Some PA 2.0 machines can only run 64-bit
kernels.  The workstations can typically run both.

The issue for user space is that 64-bit kernels can support both 32 and
64-bit applications.  This affects build scripts and make files.  The
default machine selected by config.guess for parisc64 is hppa64.
Separate applications provide 32-bit and 64-bit support in binutils, 
cc and gdb.  hppa64 selects 64-bit support when build these applications.
So, if you want a 32-bit compiler, you need to explicitly override
the default chosen by config.guess.

Historically, this occurred on parisc because the runtime differences
(on HP-UX) between 32 and 64-bit code are large, making it very difficult
to combine 32 and 64-bit support without changing a significant number
of macro defines affecting many targets.  There is still a lot of code
in gcc which depends on whether a given macro is defined or not.

In theory, it should be possible to use a single application to
generate 32 and 64-bit code on parisc for linux.  Jeff Law, who did
the 64-bit gcc implementation for parisc, said it wasn't worth the
effort to merge the two.  So far, the work to integrate the two
hasn't been done.

The situation on hpux is even more confusing, but in every case
config.guess effectively returns the assembler level that the kernel
was built with (hppa1.0, hppa1.1, hppa2.0n, hppa2.0w or hppa64) when
doing native builds.  It doesn't provide the processor family, or
specific hardware model of the build system.

In summary, config.guess has always made decisions based on kernel
configuration as this affects what can be supported in user space.
I'm sure that there are many other applications that depend on
kernel configuration in both direct and subtle ways.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

John David Anglin wrote:
> 
> Unfortunately, this decision was made years ago...
> 
> dave@mx3210:~$ uname -m
> parisc64
> dave@hiauly6:~/gnu/gcc-4.4/gcc$ uname -m
> parisc
> 
> The only significant difference between the above two machines is that
> mx3210 is running a 64-bit kernel and hiauly6 is running a 32-bit kernel.
> Both are PA 2.0 machines.  Some PA 2.0 machines can only run 64-bit
> kernels.  The workstations can typically run both.
> 
> The issue for user space is that 64-bit kernels can support both 32 and
> 64-bit applications.  This affects build scripts and make files.  The
> default machine selected by config.guess for parisc64 is hppa64.
> Separate applications provide 32-bit and 64-bit support in binutils, 
> cc and gdb.  hppa64 selects 64-bit support when build these applications.
> So, if you want a 32-bit compiler, you need to explicitly override
> the default chosen by config.guess.
> 

That's fine... the whole point is that it should not depend on kernel
CONFIG_* macros.  Depending on uname is a user-space decision, no issue
there.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

David Miller wrote:
> From: Jaswinder Singh Rajput <jaswinder@kernel.org>
> Date: Wed, 21 Jan 2009 05:34:17 +0530
> 
>> usr/include/asm-generic/fcntl.h is giving 2 'make headers_check' warnings:
>>  usr/include/asm-generic/fcntl.h:127: leaks CONFIG_64BIT to userspace where it is not valid
>>  usr/include/asm-generic/fcntl.h:149: leaks CONFIG_64BIT to userspace where it is not valid
>>
>> usr/include/asm-generic/fcntl.h:
> ...
>> #ifndef CONFIG_64BIT will always be true for userspace. So what is the use of #ifndef CONFIG_64BIT ?
> 
> Good catch.
> 
> This file needs to test for 64-bit'ness using some non-CONFIG_*
> test.  And the standard built-in CPP macros which can be used
> to check for that are different on every platform.
> 

There are a few ways to check for 64-bitness that are 
platform-independent, unfortunately each of them have drawbacks.

a) the gcc-specific way:

	#if __SIZEOF_POINTER__ == 8

    or

	#ifdef __LP64__

    or any other number of variants.

    It has the obvious disadvantage of being gcc-specific, although
    it seems rather likely that other Linux-supporting compilers might
    also define these macros.

b) the standard C way:

	#include <limits.h>

	#if LONG_MAX > 2147483647L

    it has the obvious disadvantage of needing to include <limits.h>...

We can, of course, also do our own thing as DaveM suggested; this is 
probably the best option.  FWIW, glibc has _WORDSIZE which is equivalent 
to what we call BITS_PER_LONG.  We might as well export it under that 
name (presumably under #ifndef).

	-hpa

Re: Confusion in usr/include/asm-generic/fcntl.h

[H. Peter Anvin]

H. Peter Anvin wrote:
> We can, of course, also do our own thing as DaveM suggested

s/DaveM/Arnd/ for that statement, sorry...

	-hpa

Re: Confusion in usr/include/asm-generic/fcntl.h

[Jaswinder Singh Rajput]

On Tue, 2009-01-20 at 16:48 -0800, H. Peter Anvin wrote:
> David Miller wrote:
> > From: Jaswinder Singh Rajput <jaswinder@kernel.org>
> > Date: Wed, 21 Jan 2009 05:34:17 +0530
> > 
> >> usr/include/asm-generic/fcntl.h is giving 2 'make headers_check' warnings:
> >>  usr/include/asm-generic/fcntl.h:127: leaks CONFIG_64BIT to userspace where it is not valid
> >>  usr/include/asm-generic/fcntl.h:149: leaks CONFIG_64BIT to userspace where it is not valid
> >>
> >> usr/include/asm-generic/fcntl.h:
> > ...
> >> #ifndef CONFIG_64BIT will always be true for userspace. So what is the use of #ifndef CONFIG_64BIT ?
> > 
> > Good catch.
> > 
> > This file needs to test for 64-bit'ness using some non-CONFIG_*
> > test.  And the standard built-in CPP macros which can be used
> > to check for that are different on every platform.
> > 
> 
> There are a few ways to check for 64-bitness that are 
> platform-independent, unfortunately each of them have drawbacks.
> 

So who wins the race for CONFIG_64BIT and will be right candidate for
usr/include/asm-generic/fcntl.h:

1. #if BITS_PER_LONG == 64

OR

2. #if __BITS_PER_LONG == 64

OR

3. #ifdef __LP64__

OR

4. #if __SIZEOF_POINTER__ == 8

OR

5. #if LONG_MAX > 2147483647L

OR

6. #ifdef __64BIT


Thanks
--
JSR

Re: Confusion in usr/include/asm-generic/fcntl.h

[Arnd Bergmann]

On Friday 23 January 2009, Jaswinder Singh Rajput wrote:
> 
> So who wins the race for CONFIG_64BIT and will be right candidate for
> usr/include/asm-generic/fcntl.h:
> 
> 1. #if BITS_PER_LONG == 64

namespace pollution => doesn't work

> OR
> 
> 2. #if __BITS_PER_LONG == 64

my suggestion, looks good

> 
> OR
> 
> 3. #ifdef __LP64__

Should work, but not sure if all compilers get it right

> 
> OR
> 
> 4. #if __SIZEOF_POINTER__ == 8

Never heard of this, but seems fine, probably __SIZEOF_LONG__
would be more logical (but with identical results)

> 
> OR
> 
> 5. #if LONG_MAX > 2147483647L

requires #include <stdint.h>, which pollutes the namespace => doesn't work


> OR
> 
> 6. #ifdef __64BIT

Dangerous, as explained.

	Arnd <><

Re: Confusion in usr/include/asm-generic/fcntl.h

[Jaswinder Singh Rajput]

On Mon, 2009-01-26 at 16:53 +0100, Arnd Bergmann wrote:
> > 
> > 2. #if __BITS_PER_LONG == 64
> 
> my suggestion, looks good

Thanks for confirmation, now I will prepare patchset based on
__BITS_PER_LONG :-)

--
JSR