Re: [PATCH v4 02/20] powerpc: Use generic fallocate compatibility syscall

Le 12/09/2022 à 11:57, Arnd Bergmann a écrit :

On Mon, Sep 12, 2022, at 10:38 AM, Nicholas Piggin wrote:

quoted

On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:

quoted

The powerpc fallocate compat syscall handler is identical to the
generic implementation provided by commit 59c10c52f573f ("riscv:
compat: syscall: Add compat_sys_call_table implementation"), and as
such can be removed in favour of the generic implementation.

A future patch series will replace more architecture-defined syscall
handlers with generic implementations, dependent on introducing generic
implementations that are compatible with powerpc and arm's parameter
reorderings.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rohan McLure <redacted>
---
V1 -> V2: Remove arch-specific fallocate handler.
V2 -> V3: Remove generic fallocate prototype. Move to beginning of
series.
---

quoted

quoted

@@ -16,6 +16,11 @@ typedef u16		compat_ipc_pid_t;
  #include <asm-generic/compat.h>
  
  #ifdef __BIG_ENDIAN__
+#define compat_arg_u64(name)		u32  name##_hi, u32  name##_lo
+#define compat_arg_u64_dual(name)	u32, name##_hi, u32, name##_lo
+#define compat_arg_u64_glue(name)	(((u64)name##_lo & 0xffffffffUL) | \
+					 ((u64)name##_hi << 32))

Is there a reason not to put this in asm-generic/compat.h?

Possibly you want to put this with the other compat definitions and
above the asm-generic include. The generic header expects the arch to
include it after defining what it wants to override.

Yes, makes sense. I think the riscv people added this to asm-generic,
they tried to do only the minimal parts.

In theory, any architecture could have its own calling conventions
for each syscall and have them in the opposite order for one
endianess. I checked the seven non-generic implementations of the
sys_fallocate() syscall and all except powerpc have the same
ABI as the generic one.

The powerpc difference is that in little-endian mode, only
the 'len' argument is swapped but the 'offset' argument is
still high/low:

long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
                                     u32 len1, u32 len2)
{
        return ksys_fallocate(fd, mode, ((loff_t)offset1 << 32) | offset2,
                             merge_64(len1, len2));
}

It's probably best to first fix this by using merge_64(offset1,
offset2) and allow that patch to be backported to stable kernels,
before changing it over to the generic code in a separate patch
within that series.

A related issue seems to exist in ppc_fadvise64_64(), which
uses the wrong argument order on ppc32le compat tasks, in addition
to having at least three different calling conventions across
architectures.

Do ppc32le exist at all ?

Native ppc32 is be only, and I'm not aware that ppc64 is able to run 
ppc32le compat tasks.

Christophe