linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* consolidate the flock uapi definitions
@ 2021-04-12  8:55 Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define Christoph Hellwig
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

Hi all,

currently we deal with the slight differents in the various architecture
variants of the flock and flock64 stuctures in a very cruft way.  This
series switches to just use small arch hooks and define the rest in
asm-generic and linux/compat.h instead.

Diffstat:
 arch/arm64/include/asm/compat.h        |   20 --------------------
 arch/mips/include/asm/compat.h         |   23 ++---------------------
 arch/mips/include/uapi/asm/fcntl.h     |   28 +++-------------------------
 arch/parisc/include/asm/compat.h       |   16 ----------------
 arch/powerpc/include/asm/compat.h      |   20 --------------------
 arch/s390/include/asm/compat.h         |   20 --------------------
 arch/sparc/include/asm/compat.h        |   22 +---------------------
 arch/x86/include/asm/compat.h          |   24 +++---------------------
 include/linux/compat.h                 |   31 +++++++++++++++++++++++++++++++
 include/uapi/asm-generic/fcntl.h       |   21 +++++++--------------
 tools/include/uapi/asm-generic/fcntl.h |   21 +++++++--------------
 11 files changed, 54 insertions(+), 192 deletions(-)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
@ 2021-04-12  8:55 ` Christoph Hellwig
  2021-04-12  9:55   ` Arnd Bergmann
  2021-04-12  8:55 ` [PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little Christoph Hellwig
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/uapi/asm-generic/fcntl.h       | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6ee8..fb454bb629d114 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -203,7 +203,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -216,6 +215,5 @@ struct flock64 {
 	__kernel_pid_t  l_pid;
 	__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index ac190958c98144..4a49d33ca4d55d 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -202,7 +202,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -215,6 +214,5 @@ struct flock64 {
 	__kernel_pid_t  l_pid;
 	__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define Christoph Hellwig
@ 2021-04-12  8:55 ` Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one Christoph Hellwig
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

Don't bother to define emtpty versions of the macros if the architecture
doesn't define them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/uapi/asm-generic/fcntl.h       | 12 ++++--------
 tools/include/uapi/asm-generic/fcntl.h | 12 ++++--------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index fb454bb629d114..df7ad6962dff71 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -189,22 +189,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE	1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
 	__kernel_pid_t	l_pid;
+#ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -213,7 +207,9 @@ struct flock64 {
 	__kernel_loff_t l_start;
 	__kernel_loff_t l_len;
 	__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
 	__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index 4a49d33ca4d55d..82054502b9748d 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -188,22 +188,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE	1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
 	__kernel_pid_t	l_pid;
+#ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -212,7 +206,9 @@ struct flock64 {
 	__kernel_loff_t l_start;
 	__kernel_loff_t l_len;
 	__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
 	__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little Christoph Hellwig
@ 2021-04-12  8:55 ` Christoph Hellwig
  2021-04-12  8:55 ` [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h Christoph Hellwig
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

Add a new __ARCH_FLOCK_EXTRA_SYSID macro following the style of
__ARCH_FLOCK_PAD to avoid having a separate definition just for one
architecture.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/mips/include/uapi/asm/fcntl.h     | 26 +++-----------------------
 include/uapi/asm-generic/fcntl.h       |  5 +++--
 tools/include/uapi/asm-generic/fcntl.h |  5 +++--
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
index 42e13dead54319..9e44ac810db94d 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -50,30 +50,10 @@
 #define F_SETLKW64	35
 #endif
 
-/*
- * The flavours of struct flock.  "struct flock" is the ABI compliant
- * variant.  Finally struct flock64 is the LFS variant of struct flock.	 As
- * a historic accident and inconsistence with the ABI definition it doesn't
- * contain all the same fields as struct flock.
- */
-
 #if _MIPS_SIM != _MIPS_SIM_ABI64
-
-#include <linux/types.h>
-
-struct flock {
-	short	l_type;
-	short	l_whence;
-	__kernel_off_t	l_start;
-	__kernel_off_t	l_len;
-	long	l_sysid;
-	__kernel_pid_t l_pid;
-	long	pad[4];
-};
-
-#define HAVE_ARCH_STRUCT_FLOCK
-
-#endif /* _MIPS_SIM == _MIPS_SIM_ABI32 */
+#define __ARCH_FLOCK_EXTRA_SYSID	long l_sysid;
+#define __ARCH_FLOCK_PAD		long pad[4];
+#endif
 
 #include <asm-generic/fcntl.h>
 
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index df7ad6962dff71..7e714443a8d2e3 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -188,18 +188,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE	1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+	__ARCH_FLOCK_EXTRA_SYSID
+#endif
 	__kernel_pid_t	l_pid;
 #ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
 	short  l_type;
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index 82054502b9748d..bf961a71802e0e 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -187,18 +187,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE	1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+	__ARCH_FLOCK_EXTRA_SYSID
+#endif
 	__kernel_pid_t	l_pid;
 #ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
 	short  l_type;
-- 
2.30.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-04-12  8:55 ` [PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one Christoph Hellwig
@ 2021-04-12  8:55 ` Christoph Hellwig
  2021-04-13 15:43   ` Helge Deller
  2021-04-12  8:55 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
32-bit syscall APIs, but we also need them for compat handling on 64-bit
kernels.

Given that redefining them is rather error prone, as shown by parisc
getting the opcodes wrong currently, just use the existing definitions
for the compat handling.

In theory we could try to hide them from userspace; but given that only
MIPS manages to properly do that while the asm-generic version used by
most architectures relies on a Kconfig symbol that is never set by
userspace there doesn't seem to be much of a point to even bother.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h        | 4 ----
 arch/mips/include/asm/compat.h         | 4 ----
 arch/mips/include/uapi/asm/fcntl.h     | 2 --
 arch/powerpc/include/asm/compat.h      | 4 ----
 arch/s390/include/asm/compat.h         | 4 ----
 arch/sparc/include/asm/compat.h        | 4 ----
 arch/x86/include/asm/compat.h          | 4 ----
 include/uapi/asm-generic/fcntl.h       | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 9 files changed, 30 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 23a9fb73c04ff8..a5fe4558a6ecc0 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -74,10 +74,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 65975712a22dcf..a13436e91c3938 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -60,10 +60,6 @@ struct compat_flock {
 	s32		pad[4];
 };
 
-#define F_GETLK64	33
-#define F_SETLK64	34
-#define F_SETLKW64	35
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
index 9e44ac810db94d..1769fc50d35f0e 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -44,11 +44,9 @@
 #define F_SETOWN	24	/*  for sockets. */
 #define F_GETOWN	23	/*  for sockets. */
 
-#ifndef __mips64
 #define F_GETLK64	33	/*  using 'struct flock64' */
 #define F_SETLK64	34
 #define F_SETLKW64	35
-#endif
 
 #if _MIPS_SIM != _MIPS_SIM_ABI64
 #define __ARCH_FLOCK_EXTRA_SYSID	long l_sysid;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 9191fc29e6ed11..b0f2c3f7fe45a5 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -56,10 +56,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index ea5b9c34b7be5b..e0896758779da4 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -111,10 +111,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64       12
-#define F_SETLK64       13
-#define F_SETLKW64      14    
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index b85842cda99fe0..4524997424043f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -85,10 +85,6 @@ struct compat_flock {
 	short		__unused;
 };
 
-#define F_GETLK64	12
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index be09c7eac89f09..b1691cf148be10 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -59,10 +59,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 /*
  * IA32 uses 4 byte alignment for 64 bit quantities,
  * so we need to pack this structure.
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 7e714443a8d2e3..6430a2f8023fc3 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -116,13 +116,11 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
 #define F_SETLKW64	14
 #endif
-#endif
 
 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX	15
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index bf961a71802e0e..6e16722026f39a 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -115,13 +115,11 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
 #define F_SETLKW64	14
 #endif
-#endif
 
 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX	15
-- 
2.30.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-04-12  8:55 ` [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h Christoph Hellwig
@ 2021-04-12  8:55 ` Christoph Hellwig
  2021-04-12  9:37   ` David Laight
  2021-04-12 10:03 ` consolidate the flock uapi definitions Arnd Bergmann
  2021-04-15 12:20 ` Heiko Carstens
  6 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-12  8:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-parisc,
	linuxppc-dev, linux-s390, sparclinux, linux-arch

Provide a single common definition for the compat_flock and
compat_flock64 structures using the same tricks as for the native
variants.  An extra define is added for the packing required on x86.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h   | 16 ----------------
 arch/mips/include/asm/compat.h    | 19 ++-----------------
 arch/parisc/include/asm/compat.h  | 16 ----------------
 arch/powerpc/include/asm/compat.h | 16 ----------------
 arch/s390/include/asm/compat.h    | 16 ----------------
 arch/sparc/include/asm/compat.h   | 18 +-----------------
 arch/x86/include/asm/compat.h     | 20 +++-----------------
 include/linux/compat.h            | 31 +++++++++++++++++++++++++++++++
 8 files changed, 37 insertions(+), 115 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index a5fe4558a6ecc0..6f0908606b2b51 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -66,22 +66,6 @@ struct compat_stat {
 	compat_ulong_t	__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index a13436e91c3938..7df384e445de06 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -50,23 +50,8 @@ struct compat_stat {
 	s32		st_pad4[14];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	s32		l_sysid;
-	compat_pid_t	l_pid;
-	s32		pad[4];
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
+#define __ARCH_COMPAT_FLOCK_EXTRA_SYSID		s32 l_sysid;
+#define __ARCH_COMPAT_FLOCK_PAD			s32 pad[4];
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 1a609d38f6678e..972bf8911d1126 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -54,22 +54,6 @@ struct compat_stat {
 	u32			st_spare4[3];
 };
 
-struct compat_flock {
-	short			l_type;
-	short			l_whence;
-	compat_off_t		l_start;
-	compat_off_t		l_len;
-	compat_pid_t		l_pid;
-};
-
-struct compat_flock64 {
-	short			l_type;
-	short			l_whence;
-	compat_loff_t		l_start;
-	compat_loff_t		l_len;
-	compat_pid_t		l_pid;
-};
-
 struct compat_statfs {
 	s32		f_type;
 	s32		f_bsize;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index b0f2c3f7fe45a5..e9d433bd561725 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -48,22 +48,6 @@ struct compat_stat {
 	u32		__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index e0896758779da4..5578607913b847 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -103,22 +103,6 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	u32		f_type;
 	u32		f_bsize;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 4524997424043f..16fa333f28c274 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -76,23 +76,7 @@ struct compat_stat64 {
 	unsigned int	__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
+#define __ARCH_COMPAT_FLOCK_PAD		short __unused;
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index b1691cf148be10..d8e01bf51e7a26 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -51,25 +51,11 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 /*
- * IA32 uses 4 byte alignment for 64 bit quantities,
- * so we need to pack this structure.
+ * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
+ * compat flock64 structure.
  */
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-} __attribute__((packed));
+#define __ARCH_NEED_COMPAT_FLOCK64_PACKED
 
 struct compat_statfs {
 	int		f_type;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be75360321..9d196f8a94d5ea 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -258,6 +258,37 @@ struct compat_rlimit {
 	compat_ulong_t	rlim_max;
 };
 
+#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
+#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
+#else
+#define __ARCH_COMPAT_FLOCK64_PACK
+#endif
+
+struct compat_flock {
+	short			l_type;
+	short			l_whence;
+	compat_off_t		l_start;
+	compat_off_t		l_len;
+#ifdef __ARCH_COMPAT_FLOCK_EXTRA_SYSID
+	__ARCH_COMPAT_FLOCK_EXTRA_SYSID
+#endif
+	compat_pid_t		l_pid;
+#ifdef __ARCH_COMPAT_FLOCK_PAD
+	__ARCH_COMPAT_FLOCK_PAD
+#endif
+};
+
+struct compat_flock64 {
+	short		l_type;
+	short		l_whence;
+	compat_loff_t	l_start;
+	compat_loff_t	l_len;
+	compat_pid_t	l_pid;
+#ifdef __ARCH_COMPAT_FLOCK64_PAD
+	__ARCH_COMPAT_FLOCK64_PAD
+#endif
+} __ARCH_COMPAT_FLOCK64_PACK;
+
 struct compat_rusage {
 	struct old_timeval32 ru_utime;
 	struct old_timeval32 ru_stime;
-- 
2.30.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2021-04-12  8:55 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
@ 2021-04-12  9:37   ` David Laight
  2021-04-12 10:53     ` David Laight
  0 siblings, 1 reply; 22+ messages in thread
From: David Laight @ 2021-04-12  9:37 UTC (permalink / raw)
  To: 'Christoph Hellwig',
	Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, x86, Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, linux-kernel, linux-mips,
	sparclinux, linuxppc-dev, linux-arm-kernel

From: Christoph Hellwig
> Sent: 12 April 2021 09:56
> 
> Provide a single common definition for the compat_flock and
> compat_flock64 structures using the same tricks as for the native
> variants.  An extra define is added for the packing required on x86.
> 
...
>  /*
> - * IA32 uses 4 byte alignment for 64 bit quantities,
> - * so we need to pack this structure.
> + * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
> + * compat flock64 structure.
>   */
> -struct compat_flock64 {
> -	short		l_type;
> -	short		l_whence;
> -	compat_loff_t	l_start;
> -	compat_loff_t	l_len;
> -	compat_pid_t	l_pid;
> -} __attribute__((packed));
> +#define __ARCH_NEED_COMPAT_FLOCK64_PACKED

That shouldn't need to be packed at all.
(Since the 32bit variant isn't packed.)

compat_loff_t should itself have __attribute__((aligned(4)))
probably inherited from compat_s64.
So l_start will be at offset 4 without the __packed.

I'm guessing that compat_pid_t is 16 bits?
So the native 32bit version has an unnamed 2 byte structure pad.
The 'packed' removes this pad from the compat structure.

AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just
adding an explicit pad for the implicit pad the compiler
would generate because compat_pid_t is 16 bits.

If the padding need not be named for the 64bit system calls.
(Where there is probably rather more padding all over the place.)
then it doesn't need to be named for the compat variants.

Even the mips extra padding could be removed.
F_GETLK might be expected to do a read-write of them, so
return EFAULT if not mapped.
But nothing should be testing the EFAULT is returned!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define
  2021-04-12  8:55 ` [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define Christoph Hellwig
@ 2021-04-12  9:55   ` Arnd Bergmann
  2021-04-14  6:45     ` Stephen Rothwell
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2021-04-12  9:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, the arch/x86 maintainers, Linux ARM,
	Linux Kernel Mailing List, open list:BROADCOM NVRAM DRIVER,
	Parisc List, linuxppc-dev, linux-s390, sparclinux, linux-arch

On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

The patch looks good, but I'd like to see a description for each one.
How about:

| The check was added when Stephen Rothwell created the file, but
| no architecture ever defined it.

        Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: consolidate the flock uapi definitions
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-04-12  8:55 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
@ 2021-04-12 10:03 ` Arnd Bergmann
  2021-04-12 10:22   ` David Laight
  2021-04-15 12:20 ` Heiko Carstens
  6 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2021-04-12 10:03 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	David S. Miller, the arch/x86 maintainers, Linux ARM,
	Linux Kernel Mailing List, open list:BROADCOM NVRAM DRIVER,
	Parisc List, linuxppc-dev, linux-s390, sparclinux, linux-arch

On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> currently we deal with the slight differents in the various architecture
> variants of the flock and flock64 stuctures in a very cruft way.  This
> series switches to just use small arch hooks and define the rest in
> asm-generic and linux/compat.h instead.

Nice cleanup. I can merge it through the asm-generic tree if you like,
though it's a little late just ahead of the merge window.

I would not want to change the compat_loff_t definition to compat_s64
to avoid the padding at this time, though that might be a useful cleanup
for a future cycle.

        Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: consolidate the flock uapi definitions
  2021-04-12 10:03 ` consolidate the flock uapi definitions Arnd Bergmann
@ 2021-04-12 10:22   ` David Laight
  2021-04-12 11:07     ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: David Laight @ 2021-04-12 10:22 UTC (permalink / raw)
  To: 'Arnd Bergmann', Christoph Hellwig
  Cc: linux-arch, linux-s390, Thomas Bogendoerfer, Parisc List,
	Vasily Gorbik, linuxppc-dev, Catalin Marinas, Helge Deller,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	David S. Miller, James E.J. Bottomley, Christian Borntraeger,
	sparclinux, open list:BROADCOM NVRAM DRIVER, Will Deacon,
	Heiko Carstens, Linux ARM

From: Arnd Bergmann
> Sent: 12 April 2021 11:04
> 
> On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Hi all,
> >
> > currently we deal with the slight differents in the various architecture
> > variants of the flock and flock64 stuctures in a very cruft way.  This
> > series switches to just use small arch hooks and define the rest in
> > asm-generic and linux/compat.h instead.
> 
> Nice cleanup. I can merge it through the asm-generic tree if you like,
> though it's a little late just ahead of the merge window.
> 
> I would not want to change the compat_loff_t definition to compat_s64
> to avoid the padding at this time, though that might be a useful cleanup
> for a future cycle.

Is x86 the only architecture that has 32bit and 64bit variants where
the 32bit variant aligns 64bit items on 32bit boundaries?

I've just checked MIPS and ARM, and I'm fairly sure sparc 64bit
aligns them.

Are there any others?

Might also be interesting to check whether compat_loff_t gets
used anywhere else - where the x64-64 compat code will get it
wrong.

ISTM that fixing compat_loff_t shouldn't have any fallout.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2021-04-12  9:37   ` David Laight
@ 2021-04-12 10:53     ` David Laight
  2021-04-12 11:26       ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: David Laight @ 2021-04-12 10:53 UTC (permalink / raw)
  To: 'Christoph Hellwig', 'Catalin Marinas',
	'Will Deacon', 'Thomas Bogendoerfer',
	'James E.J. Bottomley', 'Helge Deller',
	'Michael Ellerman', 'Heiko Carstens',
	'Vasily Gorbik', 'Christian Borntraeger',
	'David S. Miller', 'x86@kernel.org',
	'Arnd Bergmann'
  Cc: 'linux-arch@vger.kernel.org',
	'linux-s390@vger.kernel.org',
	'linux-parisc@vger.kernel.org',
	'linux-kernel@vger.kernel.org',
	'linux-mips@vger.kernel.org',
	'sparclinux@vger.kernel.org',
	'linuxppc-dev@lists.ozlabs.org',
	'linux-arm-kernel@lists.infradead.org'

From: David Laight
> Sent: 12 April 2021 10:37
...
> I'm guessing that compat_pid_t is 16 bits?
> So the native 32bit version has an unnamed 2 byte structure pad.
> The 'packed' removes this pad from the compat structure.
> 
> AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just
> adding an explicit pad for the implicit pad the compiler
> would generate because compat_pid_t is 16 bits.

I've just looked at the header.
compat_pid_t is 32 bits.
So Linux must have gained 32bit pids at some earlier time.
(Historically Unix pids were 16 bit - even on 32bit systems.)

Which makes the explicit pad in 'sparc' rather 'interesting'.

Actually the tail pad can just be removed from the compat
structures.
Just a comment that mips and sparc have extra fields
in the uapi header is enough.

The kernel never needs to read/write the pad.
userspace must provide the pad in case the kernel writes it.

oh - compat_loff_t is only used in a couple of other places.
neither care in any way about the alignment.
(Provided get_user() doesn't fault on a 8n+4 aligned address.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: consolidate the flock uapi definitions
  2021-04-12 10:22   ` David Laight
@ 2021-04-12 11:07     ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2021-04-12 11:07 UTC (permalink / raw)
  To: David Laight
  Cc: Christoph Hellwig, linux-arch, linux-s390, Thomas Bogendoerfer,
	Parisc List, Vasily Gorbik, linuxppc-dev, Catalin Marinas,
	Helge Deller, the arch/x86 maintainers,
	Linux Kernel Mailing List, David S. Miller, James E.J. Bottomley,
	Christian Borntraeger, sparclinux,
	open list:BROADCOM NVRAM DRIVER, Will Deacon, Heiko Carstens,
	Linux ARM

On Mon, Apr 12, 2021 at 12:22 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Arnd Bergmann
> > Sent: 12 April 2021 11:04
> >
> > On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > Hi all,
> > >
> > > currently we deal with the slight differents in the various architecture
> > > variants of the flock and flock64 stuctures in a very cruft way.  This
> > > series switches to just use small arch hooks and define the rest in
> > > asm-generic and linux/compat.h instead.
> >
> > Nice cleanup. I can merge it through the asm-generic tree if you like,
> > though it's a little late just ahead of the merge window.
> >
> > I would not want to change the compat_loff_t definition to compat_s64
> > to avoid the padding at this time, though that might be a useful cleanup
> > for a future cycle.
>
> Is x86 the only architecture that has 32bit and 64bit variants where
> the 32bit variant aligns 64bit items on 32bit boundaries?

Yes.

> ISTM that fixing compat_loff_t shouldn't have any fallout.

That is my assumption as well, but I still wouldn't take the
risk one week before the merge window.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2021-04-12 10:53     ` David Laight
@ 2021-04-12 11:26       ` Arnd Bergmann
  2021-04-12 13:11         ` David Laight
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2021-04-12 11:26 UTC (permalink / raw)
  To: David Laight
  Cc: Christoph Hellwig, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
	Michael Ellerman, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, David S. Miller, x86, linux-arch,
	linux-s390, linux-parisc, linux-kernel, linux-mips, sparclinux,
	linuxppc-dev, linux-arm-kernel

On Mon, Apr 12, 2021 at 12:54 PM David Laight <David.Laight@aculab.com> wrote:
> From: David Laight > Sent: 12 April 2021 10:37
> ...
> > I'm guessing that compat_pid_t is 16 bits?
> > So the native 32bit version has an unnamed 2 byte structure pad.
> > The 'packed' removes this pad from the compat structure.
> >
> > AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just
> > adding an explicit pad for the implicit pad the compiler
> > would generate because compat_pid_t is 16 bits.
>
> I've just looked at the header.
> compat_pid_t is 32 bits.
> So Linux must have gained 32bit pids at some earlier time.
> (Historically Unix pids were 16 bit - even on 32bit systems.)
>
> Which makes the explicit pad in 'sparc' rather 'interesting'.

I saw it was there since the sparc kernel support got merged in
linux-1.3, possibly copied from an older sunos version.

> oh - compat_loff_t is only used in a couple of other places.
> neither care in any way about the alignment.
> (Provided get_user() doesn't fault on a 8n+4 aligned address.)

Ah right, I also see that after this series it's only used in to other
places:  compat_resume_swap_area, which could also lose the
__packed annotation, and in the declaration of
compat_sys_sendfile64, where it makes no difference.

      Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2021-04-12 11:26       ` Arnd Bergmann
@ 2021-04-12 13:11         ` David Laight
  0 siblings, 0 replies; 22+ messages in thread
From: David Laight @ 2021-04-12 13:11 UTC (permalink / raw)
  To: 'Arnd Bergmann'
  Cc: Christoph Hellwig, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
	Michael Ellerman, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, David S. Miller, x86, linux-arch,
	linux-s390, linux-parisc, linux-kernel, linux-mips, sparclinux,
	linuxppc-dev, linux-arm-kernel

From: Arnd Bergmann
> Sent: 12 April 2021 12:26
> 
> On Mon, Apr 12, 2021 at 12:54 PM David Laight <David.Laight@aculab.com> wrote:
> > From: David Laight > Sent: 12 April 2021 10:37
> > ...
> > > I'm guessing that compat_pid_t is 16 bits?
> > > So the native 32bit version has an unnamed 2 byte structure pad.
> > > The 'packed' removes this pad from the compat structure.
> > >
> > > AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just
> > > adding an explicit pad for the implicit pad the compiler
> > > would generate because compat_pid_t is 16 bits.
> >
> > I've just looked at the header.
> > compat_pid_t is 32 bits.
> > So Linux must have gained 32bit pids at some earlier time.
> > (Historically Unix pids were 16 bit - even on 32bit systems.)
> >
> > Which makes the explicit pad in 'sparc' rather 'interesting'.
> 
> I saw it was there since the sparc kernel support got merged in
> linux-1.3, possibly copied from an older sunos version.

Which had a 16bit pid when I used it.
So this is a bug in the sparc merge!

The explicit 'short' pad could be removed from the 64bit variant
because there are always 4 bytes of pad after l_pid.
But it does extend the application structure on 32bit sparc so must
remain in the uapi header.
It doesn't need to be in the 'compat' definition.

> > oh - compat_loff_t is only used in a couple of other places.
> > neither care in any way about the alignment.
> > (Provided get_user() doesn't fault on a 8n+4 aligned address.)
> 
> Ah right, I also see that after this series it's only used in to other
> places:  compat_resume_swap_area, which could also lose the
> __packed annotation,

That structure just defines 0 and 8, the structure size doesn't
matter and the offsets are 'passed to' get_user() so byte
accesses aren't performed.

> and in the declaration of
> compat_sys_sendfile64, where it makes no difference.

Which should probably use get_user() rather than copy_from_user().

Although some architectures may need fallback code for
misaligned get_user() ?
Or is there a general 'cop out' that structures passed to the
kernel are required to be correctly aligned.
They should be aligned unless the kernel is 'playing games'
like reading 'struct pollfd' as a 64bit item.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h
  2021-04-12  8:55 ` [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h Christoph Hellwig
@ 2021-04-13 15:43   ` Helge Deller
  2021-04-13 15:46     ` Christoph Hellwig
  0 siblings, 1 reply; 22+ messages in thread
From: Helge Deller @ 2021-04-13 15:43 UTC (permalink / raw)
  To: Christoph Hellwig, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, James E.J. Bottomley, Heiko Carstens,
	Vasily Gorbik, Christian Borntraeger, David S. Miller,
	Arnd Bergmann
  Cc: linux-kernel, linux-mips

On 4/12/21 10:55 AM, Christoph Hellwig wrote:
> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
> kernels.

Ok.

> Given that redefining them is rather error prone, as shown by parisc
> getting the opcodes wrong currently, just use the existing definitions
> for the compat handling.

Can you please show me where parisc gets it currently wrong?
I'm somehow blind to see it and of course I'd like to fix it in
stable kernel series if that's true...

Other than that, thanks for this cleanup!
You may add:

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

Thanks!
Helge


> In theory we could try to hide them from userspace; but given that only
> MIPS manages to properly do that while the asm-generic version used by
> most architectures relies on a Kconfig symbol that is never set by
> userspace there doesn't seem to be much of a point to even bother.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   arch/arm64/include/asm/compat.h        | 4 ----
>   arch/mips/include/asm/compat.h         | 4 ----
>   arch/mips/include/uapi/asm/fcntl.h     | 2 --
>   arch/powerpc/include/asm/compat.h      | 4 ----
>   arch/s390/include/asm/compat.h         | 4 ----
>   arch/sparc/include/asm/compat.h        | 4 ----
>   arch/x86/include/asm/compat.h          | 4 ----
>   include/uapi/asm-generic/fcntl.h       | 2 --
>   tools/include/uapi/asm-generic/fcntl.h | 2 --
>   9 files changed, 30 deletions(-)
>
> diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
> index 23a9fb73c04ff8..a5fe4558a6ecc0 100644
> --- a/arch/arm64/include/asm/compat.h
> +++ b/arch/arm64/include/asm/compat.h
> @@ -74,10 +74,6 @@ struct compat_flock {
>   	compat_pid_t	l_pid;
>   };
>
> -#define F_GETLK64	12	/*  using 'struct flock64' */
> -#define F_SETLK64	13
> -#define F_SETLKW64	14
> -
>   struct compat_flock64 {
>   	short		l_type;
>   	short		l_whence;
> diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
> index 65975712a22dcf..a13436e91c3938 100644
> --- a/arch/mips/include/asm/compat.h
> +++ b/arch/mips/include/asm/compat.h
> @@ -60,10 +60,6 @@ struct compat_flock {
>   	s32		pad[4];
>   };
>
> -#define F_GETLK64	33
> -#define F_SETLK64	34
> -#define F_SETLKW64	35
> -
>   struct compat_flock64 {
>   	short		l_type;
>   	short		l_whence;
> diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
> index 9e44ac810db94d..1769fc50d35f0e 100644
> --- a/arch/mips/include/uapi/asm/fcntl.h
> +++ b/arch/mips/include/uapi/asm/fcntl.h
> @@ -44,11 +44,9 @@
>   #define F_SETOWN	24	/*  for sockets. */
>   #define F_GETOWN	23	/*  for sockets. */
>
> -#ifndef __mips64
>   #define F_GETLK64	33	/*  using 'struct flock64' */
>   #define F_SETLK64	34
>   #define F_SETLKW64	35
> -#endif
>
>   #if _MIPS_SIM != _MIPS_SIM_ABI64
>   #define __ARCH_FLOCK_EXTRA_SYSID	long l_sysid;
> diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
> index 9191fc29e6ed11..b0f2c3f7fe45a5 100644
> --- a/arch/powerpc/include/asm/compat.h
> +++ b/arch/powerpc/include/asm/compat.h
> @@ -56,10 +56,6 @@ struct compat_flock {
>   	compat_pid_t	l_pid;
>   };
>
> -#define F_GETLK64	12	/*  using 'struct flock64' */
> -#define F_SETLK64	13
> -#define F_SETLKW64	14
> -
>   struct compat_flock64 {
>   	short		l_type;
>   	short		l_whence;
> diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
> index ea5b9c34b7be5b..e0896758779da4 100644
> --- a/arch/s390/include/asm/compat.h
> +++ b/arch/s390/include/asm/compat.h
> @@ -111,10 +111,6 @@ struct compat_flock {
>   	compat_pid_t	l_pid;
>   };
>
> -#define F_GETLK64       12
> -#define F_SETLK64       13
> -#define F_SETLKW64      14
> -
>   struct compat_flock64 {
>   	short		l_type;
>   	short		l_whence;
> diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
> index b85842cda99fe0..4524997424043f 100644
> --- a/arch/sparc/include/asm/compat.h
> +++ b/arch/sparc/include/asm/compat.h
> @@ -85,10 +85,6 @@ struct compat_flock {
>   	short		__unused;
>   };
>
> -#define F_GETLK64	12
> -#define F_SETLK64	13
> -#define F_SETLKW64	14
> -
>   struct compat_flock64 {
>   	short		l_type;
>   	short		l_whence;
> diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
> index be09c7eac89f09..b1691cf148be10 100644
> --- a/arch/x86/include/asm/compat.h
> +++ b/arch/x86/include/asm/compat.h
> @@ -59,10 +59,6 @@ struct compat_flock {
>   	compat_pid_t	l_pid;
>   };
>
> -#define F_GETLK64	12	/*  using 'struct flock64' */
> -#define F_SETLK64	13
> -#define F_SETLKW64	14
> -
>   /*
>    * IA32 uses 4 byte alignment for 64 bit quantities,
>    * so we need to pack this structure.
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index 7e714443a8d2e3..6430a2f8023fc3 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -116,13 +116,11 @@
>   #define F_GETSIG	11	/* for sockets. */
>   #endif
>
> -#ifndef CONFIG_64BIT
>   #ifndef F_GETLK64
>   #define F_GETLK64	12	/*  using 'struct flock64' */
>   #define F_SETLK64	13
>   #define F_SETLKW64	14
>   #endif
> -#endif
>
>   #ifndef F_SETOWN_EX
>   #define F_SETOWN_EX	15
> diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
> index bf961a71802e0e..6e16722026f39a 100644
> --- a/tools/include/uapi/asm-generic/fcntl.h
> +++ b/tools/include/uapi/asm-generic/fcntl.h
> @@ -115,13 +115,11 @@
>   #define F_GETSIG	11	/* for sockets. */
>   #endif
>
> -#ifndef CONFIG_64BIT
>   #ifndef F_GETLK64
>   #define F_GETLK64	12	/*  using 'struct flock64' */
>   #define F_SETLK64	13
>   #define F_SETLKW64	14
>   #endif
> -#endif
>
>   #ifndef F_SETOWN_EX
>   #define F_SETOWN_EX	15
>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h
  2021-04-13 15:43   ` Helge Deller
@ 2021-04-13 15:46     ` Christoph Hellwig
  2021-04-13 15:58       ` Helge Deller
  0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2021-04-13 15:46 UTC (permalink / raw)
  To: Helge Deller
  Cc: Christoph Hellwig, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, James E.J. Bottomley, Heiko Carstens,
	Vasily Gorbik, Christian Borntraeger, David S. Miller,
	Arnd Bergmann, linux-kernel, linux-mips

On Tue, Apr 13, 2021 at 05:43:18PM +0200, Helge Deller wrote:
> On 4/12/21 10:55 AM, Christoph Hellwig wrote:
>> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
>> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
>> kernels.
>
> Ok.
>
>> Given that redefining them is rather error prone, as shown by parisc
>> getting the opcodes wrong currently, just use the existing definitions
>> for the compat handling.
>
> Can you please show me where parisc gets it currently wrong?
> I'm somehow blind to see it and of course I'd like to fix it in
> stable kernel series if that's true...

Looking again I think my comment was wrong, I thought parisc would
fall back to the asm-generic version with different code points for
the compat case, but parisc actually already defines the constants
unconditionally like this patch does for everyone else.  So I'll
retract that part of the comment.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h
  2021-04-13 15:46     ` Christoph Hellwig
@ 2021-04-13 15:58       ` Helge Deller
  0 siblings, 0 replies; 22+ messages in thread
From: Helge Deller @ 2021-04-13 15:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, David S. Miller, Arnd Bergmann,
	linux-kernel, linux-mips

On 4/13/21 5:46 PM, Christoph Hellwig wrote:
> On Tue, Apr 13, 2021 at 05:43:18PM +0200, Helge Deller wrote:
>> On 4/12/21 10:55 AM, Christoph Hellwig wrote:
>>> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
>>> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
>>> kernels.
>>
>> Ok.
>>
>>> Given that redefining them is rather error prone, as shown by parisc
>>> getting the opcodes wrong currently, just use the existing definitions
>>> for the compat handling.
>>
>> Can you please show me where parisc gets it currently wrong?
>> I'm somehow blind to see it and of course I'd like to fix it in
>> stable kernel series if that's true...
>
> Looking again I think my comment was wrong, I thought parisc would
> fall back to the asm-generic version with different code points for
> the compat case, but parisc actually already defines the constants
> unconditionally like this patch does for everyone else.  So I'll
> retract that part of the comment.

Ok, good.
Thanks for checking!

Helge

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define
  2021-04-12  9:55   ` Arnd Bergmann
@ 2021-04-14  6:45     ` Stephen Rothwell
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Rothwell @ 2021-04-14  6:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
	Michael Ellerman, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, David S. Miller, the arch/x86 maintainers,
	Linux ARM, Linux Kernel Mailing List,
	open list:BROADCOM NVRAM DRIVER, Parisc List, linuxppc-dev,
	linux-s390, sparclinux, linux-arch

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

Hi Arnd,

On Mon, 12 Apr 2021 11:55:41 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>  
> 
> The patch looks good, but I'd like to see a description for each one.
> How about:
> 
> | The check was added when Stephen Rothwell created the file, but
> | no architecture ever defined it.

Actually it was used by the xtensa architecture until Dec, 2006.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: consolidate the flock uapi definitions
  2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-04-12 10:03 ` consolidate the flock uapi definitions Arnd Bergmann
@ 2021-04-15 12:20 ` Heiko Carstens
  6 siblings, 0 replies; 22+ messages in thread
From: Heiko Carstens @ 2021-04-15 12:20 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Vasily Gorbik, Christian Borntraeger, David S. Miller, x86,
	Arnd Bergmann, linux-arm-kernel, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, linux-s390, sparclinux, linux-arch

On Mon, Apr 12, 2021 at 10:55:40AM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> currently we deal with the slight differents in the various architecture
> variants of the flock and flock64 stuctures in a very cruft way.  This
> series switches to just use small arch hooks and define the rest in
> asm-generic and linux/compat.h instead.
> 
> Diffstat:
>  arch/arm64/include/asm/compat.h        |   20 --------------------
>  arch/mips/include/asm/compat.h         |   23 ++---------------------
>  arch/mips/include/uapi/asm/fcntl.h     |   28 +++-------------------------
>  arch/parisc/include/asm/compat.h       |   16 ----------------
>  arch/powerpc/include/asm/compat.h      |   20 --------------------
>  arch/s390/include/asm/compat.h         |   20 --------------------
>  arch/sparc/include/asm/compat.h        |   22 +---------------------
>  arch/x86/include/asm/compat.h          |   24 +++---------------------
>  include/linux/compat.h                 |   31 +++++++++++++++++++++++++++++++
>  include/uapi/asm-generic/fcntl.h       |   21 +++++++--------------
>  tools/include/uapi/asm-generic/fcntl.h |   21 +++++++--------------
>  11 files changed, 54 insertions(+), 192 deletions(-)

for the s390 bits:
Acked-by: Heiko Carstens <hca@linux.ibm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2022-01-31  6:49 consolidate the compat fcntl definitions v2 Christoph Hellwig
@ 2022-01-31  6:49 ` Christoph Hellwig
  0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-01-31  6:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Guo Ren, x86, linux-arm-kernel, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, linux-s390, sparclinux, linux-arch

Provide a single common definition for the compat_flock and
compat_flock64 structures using the same tricks as for the native
variants.  Another extra define is added for the packing required on
x86.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h   | 16 ----------------
 arch/mips/include/asm/compat.h    | 19 ++-----------------
 arch/parisc/include/asm/compat.h  | 16 ----------------
 arch/powerpc/include/asm/compat.h | 16 ----------------
 arch/s390/include/asm/compat.h    | 16 ----------------
 arch/sparc/include/asm/compat.h   | 18 +-----------------
 arch/x86/include/asm/compat.h     | 20 +++-----------------
 include/linux/compat.h            | 31 +++++++++++++++++++++++++++++++
 8 files changed, 37 insertions(+), 115 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 2763287654081..e0faec1984a1c 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -65,22 +65,6 @@ struct compat_stat {
 	compat_ulong_t	__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 6a350c1f70d7e..6d6e5a451f4d9 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -55,23 +55,8 @@ struct compat_stat {
 	s32		st_pad4[14];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	s32		l_sysid;
-	compat_pid_t	l_pid;
-	s32		pad[4];
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
+#define __ARCH_COMPAT_FLOCK_EXTRA_SYSID		s32 l_sysid;
+#define __ARCH_COMPAT_FLOCK_PAD			s32 pad[4];
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index c04f5a637c390..a1e4534d80509 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -53,22 +53,6 @@ struct compat_stat {
 	u32			st_spare4[3];
 };
 
-struct compat_flock {
-	short			l_type;
-	short			l_whence;
-	compat_off_t		l_start;
-	compat_off_t		l_len;
-	compat_pid_t		l_pid;
-};
-
-struct compat_flock64 {
-	short			l_type;
-	short			l_whence;
-	compat_loff_t		l_start;
-	compat_loff_t		l_len;
-	compat_pid_t		l_pid;
-};
-
 struct compat_statfs {
 	s32		f_type;
 	s32		f_bsize;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 83d8f70779cbc..5ef3c7c83c343 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -44,22 +44,6 @@ struct compat_stat {
 	u32		__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 0f14b3188b1bb..07f04d37068b6 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -102,22 +102,6 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	u32		f_type;
 	u32		f_bsize;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 108078751bb5a..d78fb44942e0f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -75,23 +75,7 @@ struct compat_stat64 {
 	unsigned int	__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
+#define __ARCH_COMPAT_FLOCK_PAD		short __unused;
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 8d19a212f4f26..de794d8958663 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -50,25 +50,11 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 /*
- * IA32 uses 4 byte alignment for 64 bit quantities,
- * so we need to pack this structure.
+ * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
+ * compat flock64 structure.
  */
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-} __attribute__((packed));
+#define __ARCH_NEED_COMPAT_FLOCK64_PACKED
 
 struct compat_statfs {
 	int		f_type;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 1c758b0e03598..a0481fe6c5d51 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -258,6 +258,37 @@ struct compat_rlimit {
 	compat_ulong_t	rlim_max;
 };
 
+#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
+#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
+#else
+#define __ARCH_COMPAT_FLOCK64_PACK
+#endif
+
+struct compat_flock {
+	short			l_type;
+	short			l_whence;
+	compat_off_t		l_start;
+	compat_off_t		l_len;
+#ifdef __ARCH_COMPAT_FLOCK_EXTRA_SYSID
+	__ARCH_COMPAT_FLOCK_EXTRA_SYSID
+#endif
+	compat_pid_t		l_pid;
+#ifdef __ARCH_COMPAT_FLOCK_PAD
+	__ARCH_COMPAT_FLOCK_PAD
+#endif
+};
+
+struct compat_flock64 {
+	short		l_type;
+	short		l_whence;
+	compat_loff_t	l_start;
+	compat_loff_t	l_len;
+	compat_pid_t	l_pid;
+#ifdef __ARCH_COMPAT_FLOCK64_PAD
+	__ARCH_COMPAT_FLOCK64_PAD
+#endif
+} __ARCH_COMPAT_FLOCK64_PACK;
+
 struct compat_rusage {
 	struct old_timeval32 ru_utime;
 	struct old_timeval32 ru_stime;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2022-01-11  8:35 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
@ 2022-01-11  9:52   ` David Laight
  0 siblings, 0 replies; 22+ messages in thread
From: David Laight @ 2022-01-11  9:52 UTC (permalink / raw)
  To: 'Christoph Hellwig', Arnd Bergmann
  Cc: Guo Ren, x86, linux-arm-kernel, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, linux-s390, sparclinux, linux-arch

From: Christoph Hellwig
> Sent: 11 January 2022 08:35
> 
> Provide a single common definition for the compat_flock and
> compat_flock64 structures using the same tricks as for the native
> variants.  Another extra define is added for the packing required on
> x86.
> 
...
>  /*
> - * IA32 uses 4 byte alignment for 64 bit quantities,
> - * so we need to pack this structure.
> + * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
> + * compat flock64 structure.
>   */
...
> +#define __ARCH_NEED_COMPAT_FLOCK64_PACKED

Maybe:
#define __ARCH_COMPAT_FLOCK64_ATTR (packed),(aligned(4)

...
Delete this bit:
> +#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
> +#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
> +#else
> +#define __ARCH_COMPAT_FLOCK64_PACK
> +#endif
...
> +struct compat_flock64 {
> +	short		l_type;
> +	short		l_whence;
> +	compat_loff_t	l_start;
> +	compat_loff_t	l_len;
> +	compat_pid_t	l_pid;
> +#ifdef __ARCH_COMPAT_FLOCK64_PAD
> +	__ARCH_COMPAT_FLOCK64_PAD
> +#endif
> +} __ARCH_COMPAT_FLOCK64_PACK;

then:
#ifdef __ARCH_COMPAT_FLOCK64_ATTR
} __attribute__(__ARCH_COMPAT_FLOCK64_ATTR);
#else
};
#endif

Makes it a bit more like the xxx_PAD bits.
Although the trailing ; does cause a bit of grief.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
  2022-01-11  8:35 consolidate the compat fcntl definitions Christoph Hellwig
@ 2022-01-11  8:35 ` Christoph Hellwig
  2022-01-11  9:52   ` David Laight
  0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Guo Ren, x86, linux-arm-kernel, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, linux-s390, sparclinux, linux-arch

Provide a single common definition for the compat_flock and
compat_flock64 structures using the same tricks as for the native
variants.  Another extra define is added for the packing required on
x86.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h   | 16 ----------------
 arch/mips/include/asm/compat.h    | 19 ++-----------------
 arch/parisc/include/asm/compat.h  | 16 ----------------
 arch/powerpc/include/asm/compat.h | 16 ----------------
 arch/s390/include/asm/compat.h    | 16 ----------------
 arch/sparc/include/asm/compat.h   | 18 +-----------------
 arch/x86/include/asm/compat.h     | 20 +++-----------------
 include/linux/compat.h            | 31 +++++++++++++++++++++++++++++++
 8 files changed, 37 insertions(+), 115 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 2763287654081..e0faec1984a1c 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -65,22 +65,6 @@ struct compat_stat {
 	compat_ulong_t	__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 6a350c1f70d7e..6d6e5a451f4d9 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -55,23 +55,8 @@ struct compat_stat {
 	s32		st_pad4[14];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	s32		l_sysid;
-	compat_pid_t	l_pid;
-	s32		pad[4];
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
+#define __ARCH_COMPAT_FLOCK_EXTRA_SYSID		s32 l_sysid;
+#define __ARCH_COMPAT_FLOCK_PAD			s32 pad[4];
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index c04f5a637c390..a1e4534d80509 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -53,22 +53,6 @@ struct compat_stat {
 	u32			st_spare4[3];
 };
 
-struct compat_flock {
-	short			l_type;
-	short			l_whence;
-	compat_off_t		l_start;
-	compat_off_t		l_len;
-	compat_pid_t		l_pid;
-};
-
-struct compat_flock64 {
-	short			l_type;
-	short			l_whence;
-	compat_loff_t		l_start;
-	compat_loff_t		l_len;
-	compat_pid_t		l_pid;
-};
-
 struct compat_statfs {
 	s32		f_type;
 	s32		f_bsize;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 83d8f70779cbc..5ef3c7c83c343 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -44,22 +44,6 @@ struct compat_stat {
 	u32		__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 0f14b3188b1bb..07f04d37068b6 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -102,22 +102,6 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	u32		f_type;
 	u32		f_bsize;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 108078751bb5a..d78fb44942e0f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -75,23 +75,7 @@ struct compat_stat64 {
 	unsigned int	__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
+#define __ARCH_COMPAT_FLOCK_PAD		short __unused;
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 8d19a212f4f26..de794d8958663 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -50,25 +50,11 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 /*
- * IA32 uses 4 byte alignment for 64 bit quantities,
- * so we need to pack this structure.
+ * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
+ * compat flock64 structure.
  */
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-} __attribute__((packed));
+#define __ARCH_NEED_COMPAT_FLOCK64_PACKED
 
 struct compat_statfs {
 	int		f_type;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 1c758b0e03598..a0481fe6c5d51 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -258,6 +258,37 @@ struct compat_rlimit {
 	compat_ulong_t	rlim_max;
 };
 
+#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
+#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
+#else
+#define __ARCH_COMPAT_FLOCK64_PACK
+#endif
+
+struct compat_flock {
+	short			l_type;
+	short			l_whence;
+	compat_off_t		l_start;
+	compat_off_t		l_len;
+#ifdef __ARCH_COMPAT_FLOCK_EXTRA_SYSID
+	__ARCH_COMPAT_FLOCK_EXTRA_SYSID
+#endif
+	compat_pid_t		l_pid;
+#ifdef __ARCH_COMPAT_FLOCK_PAD
+	__ARCH_COMPAT_FLOCK_PAD
+#endif
+};
+
+struct compat_flock64 {
+	short		l_type;
+	short		l_whence;
+	compat_loff_t	l_start;
+	compat_loff_t	l_len;
+	compat_pid_t	l_pid;
+#ifdef __ARCH_COMPAT_FLOCK64_PAD
+	__ARCH_COMPAT_FLOCK64_PAD
+#endif
+} __ARCH_COMPAT_FLOCK64_PACK;
+
 struct compat_rusage {
 	struct old_timeval32 ru_utime;
 	struct old_timeval32 ru_stime;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-01-31  6:50 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12  8:55 consolidate the flock uapi definitions Christoph Hellwig
2021-04-12  8:55 ` [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define Christoph Hellwig
2021-04-12  9:55   ` Arnd Bergmann
2021-04-14  6:45     ` Stephen Rothwell
2021-04-12  8:55 ` [PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little Christoph Hellwig
2021-04-12  8:55 ` [PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one Christoph Hellwig
2021-04-12  8:55 ` [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h Christoph Hellwig
2021-04-13 15:43   ` Helge Deller
2021-04-13 15:46     ` Christoph Hellwig
2021-04-13 15:58       ` Helge Deller
2021-04-12  8:55 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
2021-04-12  9:37   ` David Laight
2021-04-12 10:53     ` David Laight
2021-04-12 11:26       ` Arnd Bergmann
2021-04-12 13:11         ` David Laight
2021-04-12 10:03 ` consolidate the flock uapi definitions Arnd Bergmann
2021-04-12 10:22   ` David Laight
2021-04-12 11:07     ` Arnd Bergmann
2021-04-15 12:20 ` Heiko Carstens
2022-01-11  8:35 consolidate the compat fcntl definitions Christoph Hellwig
2022-01-11  8:35 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig
2022-01-11  9:52   ` David Laight
2022-01-31  6:49 consolidate the compat fcntl definitions v2 Christoph Hellwig
2022-01-31  6:49 ` [PATCH 5/5] compat: consolidate the compat_flock{,64} definition Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).