Skip to contentSkip to conversation

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

DISCUSSION Jaswinder Singh Rajput 10 participants 23 messages Jan 21, 2009 – Jan 27, 2009 PA-RISC architecture

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

From: Jaswinder Singh Rajput <[email protected]>
Date: Wed, 21 Jan 2009 05:34:17 +0530

Quoted 5 lines

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:

...

Quoted 1 line

#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.

#2 lore raw
Quoted 8 lines · David Miller

From: Jaswinder Singh Rajput <[email protected]>
Date: Wed, 21 Jan 2009 05:34:17 +0530

Quoted 5 lines

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:

Quoted 8 lines

...

Quoted 1 line

#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 <><
#3 lore raw

From: Arnd Bergmann <[email protected]>
Date: Wed, 21 Jan 2009 01:24:25 +0100

Quoted 4 lines

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.

Quoted 4 lines

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.

maintainer
#4 lore raw

From: Arnd Bergmann <[email protected]>

Quoted 4 lines
Quoted 4 lines

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

#5 lore raw
Quoted 9 lines · Helge Deller

From: Arnd Bergmann <[email protected]>

Quoted 4 lines
Quoted 4 lines

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 <><
#6 lore raw
Quoted 11 lines · Arnd Bergmann
Quoted 9 lines · Helge Deller

From: Arnd Bergmann <[email protected]>

Quoted 4 lines
Quoted 4 lines

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.

Quoted 5 lines

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

#7 lore raw
Quoted 2 lines · Sam Ravnborg

Could we add a new symbol for this?
We know we are going to use this in several places so a simpler varia=

nt

Quoted 9 lines

would be more readable.
=20
Something like:
=20
#ifdef __64BIT
...
#endif
=20
When we define __64BIT we would use the =A0__BITS_PER_LONG =3D=3D 64 =

check.

I would prefer using the __BITS_PER_LONG =3D=3D 64 check directly, beca= use
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 <><

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · Sam Ravnborg

Could we add a new symbol for this?
We know we are going to use this in several places so a simpler var=

iant

Quoted 9 lines
Quoted 9 lines

would be more readable.
=20
Something like:
=20
#ifdef __64BIT
...
#endif
=20
When we define __64BIT we would use the =A0__BITS_PER_LONG =3D=3D 6=

4 check.

Quoted 2 lines

=20
I would prefer using the __BITS_PER_LONG =3D=3D 64 check directly, be=

cause

Quoted 5 lines

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.
=20

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

cheers, Kyle
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw
In reply to Arnd Bergmann
Quoted 7 lines · Arnd Bergmann

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

#10 lore raw
In reply to Arnd Bergmann
Quoted 11 lines · Arnd Bergmann
Quoted 10 lines · Sam Ravnborg

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

Quoted 7 lines
Quoted 1 line

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

#11 lore raw
Quoted 10 lines · Sam Ravnborg

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

maintainer
#12 lore raw
In reply to Arnd Bergmann
Quoted 10 lines · Arnd Bergmann
Quoted 8 lines · Helge Deller

From: Arnd Bergmann <[email protected]>

Quoted 4 lines
Quoted 4 lines

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.

Quoted 5 lines

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

arch/parisc/include/asm/assembly.h +11 -9
index ab7cc37..b950843 100644
@@ -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
arch/parisc/include/asm/msgbuf.h +5 -3
index fe88f26..a4520f5 100644
@@ -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 */
arch/parisc/include/asm/pdc.h +5 -7
index c584b00..f118764 100644
@@ -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);
arch/parisc/include/asm/posix_types.h +3 -1
index 00da29a..6946cdc 100644
@@ -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;
arch/parisc/include/asm/sembuf.h +2 -2
index 1e59ffd..d27e904 100644
@@ -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 */
arch/parisc/include/asm/shmbuf.h +7 -6
index 0a3eada..d133bf4 100644
@@ -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;
arch/parisc/include/asm/signal.h +1 -1
index c203563..3de6265 100644
@@ -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
arch/parisc/include/asm/swab.h +1 -1
index 3ff16c5..e78403b 100644
@@ -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__
arch/parisc/include/asm/types.h +10 -0
index 7f5a39b..14bb5bd 100644
@@ -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.  */
#13 lore raw
In reply to Arnd Bergmann

On Wed, Jan 21, 2009 at 01:24:25AM +0100, Arnd Bergmann wrote:
...

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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__.

Quoted 2 lines

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

#14 lore raw
Quoted 3 lines
Quoted 1 line

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

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
#15 lore raw
Quoted 8 lines · John David Anglin
Quoted 2 lines
Quoted 1 line

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

#16 lore raw
Quoted 3 lines

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

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
#17 lore raw
Quoted 13 lines · John David Anglin

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.

Quoted 8 lines

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

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#18 lore raw
In reply to David Miller
Quoted 8 lines · David Miller

From: Jaswinder Singh Rajput <[email protected]>
Date: Wed, 21 Jan 2009 05:34:17 +0530

Quoted 5 lines

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:

Quoted 9 lines

...

Quoted 1 line

#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

#19 lore raw
Quoted 1 line · H. Peter Anvin

We can, of course, also do our own thing as DaveM suggested

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

-hpa

#20 lore raw
In reply to H. Peter Anvin
Quoted 14 lines · H. Peter Anvin
Quoted 13 lines · David Miller

From: Jaswinder Singh Rajput <[email protected]>
Date: Wed, 21 Jan 2009 05:34:17 +0530

Quoted 5 lines

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:

...

Quoted 1 line

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

Good catch.

Quoted 8 lines
Quoted 4 lines

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

#21 lore raw
Quoted 5 lines · Jaswinder Singh Rajput

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

Quoted 3 lines

OR

2. #if __BITS_PER_LONG == 64

my suggestion, looks good

Quoted 4 lines

OR

3. #ifdef __LP64__

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

Quoted 4 lines

OR

4. #if __SIZEOF_POINTER__ == 8

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

Quoted 4 lines

OR

5. #if LONG_MAX > 2147483647L

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

Quoted 3 lines

OR

6. #ifdef __64BIT

Dangerous, as explained.

	Arnd <><
#22 lore raw
Quoted 4 lines · Arnd Bergmann
Quoted 2 lines

2. #if __BITS_PER_LONG == 64

my suggestion, looks good

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

--
JSR