On Mon, Feb 28, 2022 at 8:02 PM David Laight [off-list ref] wrote:

From: Guo Ren

quoted

Sent: 28 February 2022 11:52

On Mon, Feb 28, 2022 at 2:40 PM David Laight [off-list ref] wrote:

quoted

From: guoren@kernel.org

quoted

Sent: 27 February 2022 16:28

From: Christoph Hellwig <hch@lst.de>

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.

...

quoted

diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h

...

quoted

 /*
- * 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.
  */

...

quoted

+#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 1c758b0e0359..a0481fe6c5d5 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

...

quoted

+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;
+

Provided compat_loff_t are correctly defined with __aligned__(4)

See include/asm-generic/compat.h

typedef s64 compat_loff_t;

Only:
#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
typedef s64 __attribute__((aligned(4))) compat_s64;

So how do you think compat_loff_t could be defined with __aligned__(4)?

compat_loff_t should be compat_s64 not s64.

The same should be done for all 64bit 'compat' types.

Changing
typedef s64 compat_loff_t;
to
typedef compat_s64 compat_loff_t;

should be another patch and it affects all architectures, I don't
think we should involve it in this series.

look at kernel/power/user.c:
struct compat_resume_swap_area {
        compat_loff_t offset;
        u32 dev;
} __packed;

I thnk keep "typedef s64 compat_loff_t;" is a sensible choice for
COMPAT support patchset series.

        David

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

--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/