Skip to contentSkip to conversation

parisc: unbreak automounter support on 64-bit kernel with 32-bit userspace

PATCH In review Helge Deller 6 participants 14 messages Jan 31, 2013 – Feb 6, 2013 Kernel automounter

lore
Share & tools
lore

Helge Deller

author
#0 lore raw

PATCH 1/1 parisc: unbreak automounter support on 64-bit kernel with 32-bit userspace

Similiar to other 64 bit Linux targets autofs_wqt_t needs to be of type int which has a size of 32 bits on 32- and 64-bit parisc kernels.

Signed-off-by Helge Deller <[email protected]>
include/uapi/linux/auto_fs.h +1 -1
index 77cdba9..d551754 100644
@@ -43,7 +43,7 @@
  */
 
 #if defined(__sparc__) || defined(__mips__) || defined(__x86_64__) \
- || defined(__powerpc__) || defined(__s390__)
+ || defined(__powerpc__) || defined(__s390__) || defined(__hppa__)
 typedef unsigned int autofs_wqt_t;
 #else
 typedef unsigned long autofs_wqt_t;
#1 lore raw

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

Quoted 22 lines · Helge Deller

Similiar to other 64 bit Linux targets autofs_wqt_t needs to be of type int which has a size of 32 bits on 32- and 64-bit parisc kernels.

Signed-off-by Helge Deller <[email protected]>
include/uapi/linux/auto_fs.h +1 -2
index 77cdba9..d551754 100644
@@ -43,7 +43,7 @@
  */
 
 #if defined(__sparc__) || defined(__mips__) || defined(__x86_64__) \
- || defined(__powerpc__) || defined(__s390__)
+ || defined(__powerpc__) || defined(__s390__) || defined(__hppa__)
 typedef unsigned int autofs_wqt_t;
 #else
 typedef unsigned long autofs_wqt_t;
--

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

#2 lore raw
Quoted 20 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

Quoted 14 lines · Helge Deller

Similiar to other 64 bit Linux targets autofs_wqt_t needs to be of type int
which has a size of 32 bits on 32- and 64-bit parisc kernels.

Signed-off-by Helge Deller <[email protected]>
include/uapi/linux/auto_fs.h +0 -0
index 77cdba9..d551754 100644
@@ -43,7 +43,7 @@
 
  */
Quoted 8 lines
Quoted 8 lines
include/uapi/linux/auto_fs.h +1 -1
 #if defined(__sparc__) || defined(__mips__) || defined(__x86_64__) \
 
- || defined(__powerpc__) || defined(__s390__)
+ || defined(__powerpc__) || defined(__s390__) || defined(__hppa__)

typedef unsigned int autofs_wqt_t;
#else
typedef unsigned long autofs_wqt_t;

Wouldn't it make sense to make it an unsigned int on all platforms? This wouldn't make any difference on 32 bit platforms and would prevent that to happen on the next 32-on-64 bit platform again.

Eike

#3 lore raw
In reply to James Bottomley
Quoted 4 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

The #if list probably should be inverted, and only specific platforms should have unsigned long...

-hpa

#4 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

The #if list probably should be inverted, and only specific platforms should have unsigned long...

The whitelist is 64 bit only platforms, like ia64, isn't it? So yes, they're far less numerous.

James

#5 lore raw
Quoted 5 lines · James Bottomley
Quoted 4 lines · H. Peter Anvin
Quoted 3 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

James]

The #if list probably should be inverted, and only specific platforms should have unsigned long...

The whitelist is 64 bit only platforms, like ia64, isn't it? So yes, they're far less numerous.

Correct. It is basically the 64-bit platforms without any 32-bit userspace, like Alpha and IA64.

-hpa

#6 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · James Bottomley
Quoted 6 lines · H. Peter Anvin
Quoted 4 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

The #if list probably should be inverted, and only specific platforms should have unsigned long...

The whitelist is 64 bit only platforms, like ia64, isn't it? So yes, they're far less numerous.

Correct. It is basically the 64-bit platforms without any 32-bit userspace, like Alpha and IA64.

Arm64? Should this be a CONFIG_ARCH_CAN_HAVE_32BIT_USERSPACE that would cover this? And may e.g. be used to remove compat_ioctl on those archs, too?

Eike

#7 lore raw
Quoted 15 lines · Rolf Eike Beer
Quoted 14 lines · H. Peter Anvin
Quoted 12 lines · James Bottomley
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

The #if list probably should be inverted, and only specific platforms should have unsigned long...

The whitelist is 64 bit only platforms, like ia64, isn't it? So yes, they're far less numerous.

Quoted 7 lines
Quoted 2 lines

Correct. It is basically the 64-bit platforms without any 32-bit userspace, like Alpha and IA64.

Arm64? Should this be a CONFIG_ARCH_CAN_HAVE_32BIT_USERSPACE that would cover this? And may e.g. be used to remove compat_ioctl on those archs, too?

compat_ioctl should be CONFIG_COMPAT.

-hpa

#8 lore raw
In reply to Rolf Eike Beer
Quoted 12 lines · Rolf Eike Beer
Quoted 11 lines · H. Peter Anvin
Quoted 10 lines · James Bottomley
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · James Bottomley

[adding autofs list and maintainer for their perusal and ack, although this is an obvious fix to me

James]

The #if list probably should be inverted, and only specific platforms should have unsigned long...

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

The whitelist is 64 bit only platforms, like ia64, isn't it? So yes, they're far less numerous.

Correct. It is basically the 64-bit platforms without any 32-bit userspace, like Alpha and IA64.

Arm64?

It supports 32-bit user-space (CONFIG_COMPAT). But we missed this #ifdef for arm64. I'm not familiar with the autofs subsystem but at a first look it seems that autofs_wqt_t types are not shared with user space as part of a structure. It can come as an argument for an ioctl command but as long as we don't expect this type to hold data larger than 32-bit it should be ok.

Signature · 1 line
Catalin
author
#9 lore raw

PATCH 1/1 unbreak automounter support on 64-bit kernel with 32-bit userspace (v2)

In reply to H. Peter Anvin

automount-support is broken on the parisc architecture, because the existing #if list does not include a check for defined(__hppa__). The HPPA (parisc) architecture is similiar to other 64bit Linux targets where we have to define autofs_wqt_t (which is passed back and forth to user space) as int type which has a size of 32bit across 32 and 64bit kernels.

During the discussion on the mailing list, H. Peter Anvin suggested to invert the #if list since only specific platforms (specifically those who do not have a 32bit userspace, like IA64 and Alpha) should have autofs_wqt_t as unsigned long type.

This suggestion is probably the best way to go, since Arm64 (and maybe others?) seems to have a non-working automounter. So in the long run even for other new upcoming architectures this inverted check seem to be the best solution, since it will not require them to change this #if again (unless they are 64bit only).

Signed-off-by Helge Deller <[email protected]>
CC James Bottomley <[email protected]>
CC Catalin Marinas <[email protected]>
CC Rolf Eike Beer <[email protected]>
CC H. Peter Anvin <[email protected]>
CC Ian Kent <[email protected]>
include/uapi/linux/auto_fs.h +5 -12
index 77cdba9..bb991df 100644
@@ -28,25 +28,16 @@
 #define AUTOFS_MIN_PROTO_VERSION	AUTOFS_PROTO_VERSION
 
 /*
- * Architectures where both 32- and 64-bit binaries can be executed
- * on 64-bit kernels need this.  This keeps the structure format
- * uniform, and makes sure the wait_queue_token isn't too big to be
- * passed back down to the kernel.
- *
- * This assumes that on these architectures:
- * mode     32 bit    64 bit
- * -------------------------
- * int      32 bit    32 bit
- * long     32 bit    64 bit
- *
- * If so, 32-bit user-space code should be backwards compatible.
+ * The wait_queue_token (autofs_wqt_t) is part of a structure which is passed
+ * back to the kernel via ioctl from userspace. On architectures where 32- and
+ * 64-bit userspace binaries can be executed it's important that the size of
+ * autofs_wqt_t stays constant between 32- and 64-bit Linux kernels so that we
+ * do not break the binary ABI interface by changing the structure size.
  */
#10 lore raw

Re: [PATCH] unbreak automounter support on 64-bit kernel with 32-bit userspace (v2)

Quoted 6 lines · Helge Deller

This suggestion is probably the best way to go, since Arm64 (and maybe others?) seems to have a non-working automounter. So in the long run even for other new upcoming architectures this inverted check seem to be the best solution, since it will not require them to change this #if again (unless they are 64bit only).

... and even then they will generally work just fine.

-hpa

maintainer Acked-by on v1 1/1
#11 lore raw

Re: [PATCH] unbreak automounter support on 64-bit kernel with 32-bit userspace (v2)

In reply to Helge Deller
Quoted 52 lines · Helge Deller

automount-support is broken on the parisc architecture, because the existing #if list does not include a check for defined(__hppa__). The HPPA (parisc) architecture is similiar to other 64bit Linux targets where we have to define autofs_wqt_t (which is passed back and forth to user space) as int type which has a size of 32bit across 32 and 64bit kernels.

During the discussion on the mailing list, H. Peter Anvin suggested to invert the #if list since only specific platforms (specifically those who do not have a 32bit userspace, like IA64 and Alpha) should have autofs_wqt_t as unsigned long type.

This suggestion is probably the best way to go, since Arm64 (and maybe others?) seems to have a non-working automounter. So in the long run even for other new upcoming architectures this inverted check seem to be the best solution, since it will not require them to change this #if again (unless they are 64bit only).

Signed-off-by Helge Deller <[email protected]>
CC James Bottomley <[email protected]>
CC Catalin Marinas <[email protected]>
CC Rolf Eike Beer <[email protected]>
CC H. Peter Anvin <[email protected]>
CC Ian Kent <[email protected]>
include/uapi/linux/auto_fs.h +5 -14
index 77cdba9..bb991df 100644
@@ -28,25 +28,16 @@
 #define AUTOFS_MIN_PROTO_VERSION	AUTOFS_PROTO_VERSION
 
 /*
- * Architectures where both 32- and 64-bit binaries can be executed
- * on 64-bit kernels need this.  This keeps the structure format
- * uniform, and makes sure the wait_queue_token isn't too big to be
- * passed back down to the kernel.
- *
- * This assumes that on these architectures:
- * mode     32 bit    64 bit
- * -------------------------
- * int      32 bit    32 bit
- * long     32 bit    64 bit
- *
- * If so, 32-bit user-space code should be backwards compatible.
+ * The wait_queue_token (autofs_wqt_t) is part of a structure which is passed
+ * back to the kernel via ioctl from userspace. On architectures where 32- and
+ * 64-bit userspace binaries can be executed it's important that the size of
+ * autofs_wqt_t stays constant between 32- and 64-bit Linux kernels so that we
+ * do not break the binary ABI interface by changing the structure size.
  */
-
-#if defined(__sparc__) || defined(__mips__) || defined(__x86_64__) \

include/uapi/linux/auto_fs.h line 43

Quoted 10 lines
include/uapi/linux/auto_fs.h +3 -3
- || defined(__powerpc__) || defined(__s390__)
-typedef unsigned int autofs_wqt_t;
-#else
+#if defined(__ia64__) || defined(__alpha__) /* pure 64bit architectures */
 typedef unsigned long autofs_wqt_t;
+#else
+typedef unsigned int autofs_wqt_t;
 #endif
 
 /* Packet types */
Acked-by Ian Kent <[email protected]>
Acked-by
#12 lore raw

Re: [PATCH] unbreak automounter support on 64-bit kernel with 32-bit userspace (v2)

Quoted 38 lines · Ian Kent
Quoted 38 lines
include/uapi/linux/auto_fs.h +8 -17
index 77cdba9..bb991df 100644
@@ -28,25 +28,16 @@
  #define AUTOFS_MIN_PROTO_VERSION	AUTOFS_PROTO_VERSION
 
  /*
- * Architectures where both 32- and 64-bit binaries can be executed
- * on 64-bit kernels need this.  This keeps the structure format
- * uniform, and makes sure the wait_queue_token isn't too big to be
- * passed back down to the kernel.
- *
- * This assumes that on these architectures:
- * mode     32 bit    64 bit
- * -------------------------
- * int      32 bit    32 bit
- * long     32 bit    64 bit
- *
- * If so, 32-bit user-space code should be backwards compatible.
+ * The wait_queue_token (autofs_wqt_t) is part of a structure which is passed
+ * back to the kernel via ioctl from userspace. On architectures where 32- and
+ * 64-bit userspace binaries can be executed it's important that the size of
+ * autofs_wqt_t stays constant between 32- and 64-bit Linux kernels so that we
+ * do not break the binary ABI interface by changing the structure size.
   */
-
-#if defined(__sparc__) || defined(__mips__) || defined(__x86_64__) \
- || defined(__powerpc__) || defined(__s390__)
-typedef unsigned int autofs_wqt_t;
-#else
+#if defined(__ia64__) || defined(__alpha__) /* pure 64bit architectures */
  typedef unsigned long autofs_wqt_t;
+#else
+typedef unsigned int autofs_wqt_t;
  #endif
Quoted 4 lines

include/uapi/linux/auto_fs.h line 43

Quoted 1 line
include/uapi/linux/auto_fs.h +0 -0
  /* Packet types */
Acked-by Ian Kent <[email protected]>
Acked-by H. Peter Anvin <[email protected]>

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
Acked-by on v1 1/1
#13 lore raw

Re: [PATCH] unbreak automounter support on 64-bit kernel with 32-bit userspace (v2)

In reply to Helge Deller
Quoted 16 lines · Helge Deller

automount-support is broken on the parisc architecture, because the existing #if list does not include a check for defined(__hppa__). The HPPA (parisc) architecture is similiar to other 64bit Linux targets where we have to define autofs_wqt_t (which is passed back and forth to user space) as int type which has a size of 32bit across 32 and 64bit kernels.

During the discussion on the mailing list, H. Peter Anvin suggested to invert the #if list since only specific platforms (specifically those who do not have a 32bit userspace, like IA64 and Alpha) should have autofs_wqt_t as unsigned long type.

This suggestion is probably the best way to go, since Arm64 (and maybe others?) seems to have a non-working automounter. So in the long run even for other new upcoming architectures this inverted check seem to be the best solution, since it will not require them to change this #if again (unless they are 64bit only).

Quoted 6 lines
Signed-off-by Helge Deller <[email protected]>
CC James Bottomley <[email protected]>
CC Catalin Marinas <[email protected]>
CC Rolf Eike Beer <[email protected]>
CC H. Peter Anvin <[email protected]>
CC Ian Kent <[email protected]>
Acked-by Catalin Marinas <[email protected]>