* [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 related [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: [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
* [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 related [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 related [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 related [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
* [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 related [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 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: [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: 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: 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: 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