Re: [PATCH V3 13/22] LoongArch: Add system call support

Hi, Arnd,

On Fri, Sep 17, 2021 at 4:24 PM Arnd Bergmann [off-list ref] wrote:

On Fri, Sep 17, 2021 at 5:57 AM Huacai Chen [off-list ref] wrote:

quoted

+#define NR_syscalls (__NR_syscalls)

diff --git a/arch/loongarch/include/uapi/asm/unistd.h b/arch/loongarch/include/uapi/asm/unistd.h
new file mode 100644
index 000000000000..b344b1f91715
--- /dev/null
+++ b/arch/loongarch/include/uapi/asm/unistd.h

@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#define __ARCH_WANT_NEW_STAT
+#define __ARCH_WANT_SYS_CLONE
+#define __ARCH_WANT_SYS_CLONE3

I still think you need to remove __ARCH_WANT_NEW_STAT and
__ARCH_WANT_SYS_CLONE here.

I understand that those are needed for the transitional period when you
still need to support your existing glibc library files, but you likely still
have other kernel patches that are not part of this series, so I suggest
you add those two lines as a custom patch there until you are ready to
drop support for old libc.

The clone story:
When I sent V1 of this series, the upstream glibc (2.33) hadn't merge
the clone3 support. Now glibc 2.34 has merged clone3, so
__ARCH_WANT_SYS_CLONE seems can be removed. But I think there is
someone just download this series and suppose it can work with current
userspace. So I want to keep it for a while, until this series can be
merged.

The statx story:
The latest upstream glibc (2.34) is still like this
(sysdeps/unix/sysv/linux/fstatat64.c):
#if (__WORDSIZE == 32 \
     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
     || defined STAT_HAS_TIME32
# define FSTATAT_USE_STATX 1
#else
# define FSTATAT_USE_STATX 0
#endif

This means statx is supposed to use in 32bit systems, or 64bit systems
with 32bit timestamp (e.g. MIPS64). So I think __ARCH_WANT_NEW_STAT is
still needed.

quoted

+
+SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
+       unsigned long, prot, unsigned long, flags, unsigned long,
+       fd, off_t, offset)
+{
+       if (offset & ~PAGE_MASK)
+               return -EINVAL;
+       return ksys_mmap_pgoff(addr, len, prot, flags, fd,
+                              offset >> PAGE_SHIFT);
+}
+
+SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
+       unsigned long, prot, unsigned long, flags, unsigned long, fd,
+       unsigned long, pgoff)
+{
+       if (pgoff & (~PAGE_MASK >> 12))
+               return -EINVAL;
+
+       return ksys_mmap_pgoff(addr, len, prot, flags, fd,
+                              pgoff >> (PAGE_SHIFT - 12));
+}

sys_mmap2() is only used on 32-bit architectures, you only need
sys_mmap() here.

Ideally we'd just move those two definitions you have here into
mm/mmap.c and remove all the duplicate definitions. Maybe
you can come up with a patch to do this?

Note that some architectures use either nonstandard names,
or shift value other than 12, so those need to keep their own
versions.

OK, sys_mmap2() will be removed. But can "move sys_mmap() to
mm/mmap.c" be done by others? (I think my credit is not enough to do
this, at least now).

Huacai

      Arnd