All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] linux-user: Fix inotify on aarch64
@ 2022-01-26 20:26 Paul Brook
  2022-01-27 14:08 ` Laurent Vivier
  2022-01-27 14:10 ` Laurent Vivier
  0 siblings, 2 replies; 3+ messages in thread
From: Paul Brook @ 2022-01-26 20:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Paul Brook

The inotify implementation originally called the raw host syscalls.
Commit 3b3f24add0 changed this to use the glibc wrappers. However ifdefs
in syscall.c still test for presence of the raw syscalls.

This causes a problem on e.g. aarch64 hosts which never had the
inotify_init syscall - it had been obsoleted by inotify_init1 before
aarch64 was invented! However it does have a perfectly good glibc
implementation of inotify_wait.

Fix this by removing all the raw __NR_inotify_* tests, and instead check
CONFIG_INOTIFY, which already tests for the glibc functionality we use.

Also remove the now-pointless sys_inotify* wrappers.

Tested using x86-64 inotifywatch on aarch64 host, and vice-versa

Signed-off-by: Paul Brook <paul@nowt.org>
---
 linux-user/fd-trans.c |  5 ++---
 linux-user/syscall.c  | 50 +++++++++----------------------------------
 2 files changed, 12 insertions(+), 43 deletions(-)

diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index 6941089959..30e7b49112 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -1460,9 +1460,8 @@ TargetFdTrans target_eventfd_trans = {
     .target_to_host_data = swap_data_eventfd,
 };
 
-#if (defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)) || \
-    (defined(CONFIG_INOTIFY1) && defined(TARGET_NR_inotify_init1) && \
-     defined(__NR_inotify_init1))
+#if defined(CONFIG_INOTIFY) && (defined(TARGET_NR_inotify_init) || \
+        defined(TARGET_NR_inotify_init1))
 static abi_long host_to_target_data_inotify(void *buf, size_t len)
 {
     struct inotify_event *ev;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 56a3e17183..17cc38fe34 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -272,9 +272,6 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
 #if defined(__NR_futex_time64)
 # define __NR_sys_futex_time64 __NR_futex_time64
 #endif
-#define __NR_sys_inotify_init __NR_inotify_init
-#define __NR_sys_inotify_add_watch __NR_inotify_add_watch
-#define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch
 #define __NR_sys_statx __NR_statx
 
 #if defined(__alpha__) || defined(__x86_64__) || defined(__s390x__)
@@ -447,33 +444,6 @@ static int sys_renameat2(int oldfd, const char *old,
 
 #ifdef CONFIG_INOTIFY
 #include <sys/inotify.h>
-
-#if defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)
-static int sys_inotify_init(void)
-{
-  return (inotify_init());
-}
-#endif
-#if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
-static int sys_inotify_add_watch(int fd,const char *pathname, int32_t mask)
-{
-  return (inotify_add_watch(fd, pathname, mask));
-}
-#endif
-#if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
-static int sys_inotify_rm_watch(int fd, int32_t wd)
-{
-  return (inotify_rm_watch(fd, wd));
-}
-#endif
-#ifdef CONFIG_INOTIFY1
-#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
-static int sys_inotify_init1(int flags)
-{
-  return (inotify_init1(flags));
-}
-#endif
-#endif
 #else
 /* Userspace can usually survive runtime without inotify */
 #undef TARGET_NR_inotify_init
@@ -12263,35 +12233,35 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_futex_time64:
         return do_futex_time64(cpu, arg1, arg2, arg3, arg4, arg5, arg6);
 #endif
-#if defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)
+#ifdef CONFIG_INOTIFY
+#if defined(TARGET_NR_inotify_init)
     case TARGET_NR_inotify_init:
-        ret = get_errno(sys_inotify_init());
+        ret = get_errno(inotify_init());
         if (ret >= 0) {
             fd_trans_register(ret, &target_inotify_trans);
         }
         return ret;
 #endif
-#ifdef CONFIG_INOTIFY1
-#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
+#if defined(TARGET_NR_inotify_init1) && defined(CONFIG_INOTIFY1)
     case TARGET_NR_inotify_init1:
-        ret = get_errno(sys_inotify_init1(target_to_host_bitmask(arg1,
+        ret = get_errno(inotify_init1(target_to_host_bitmask(arg1,
                                           fcntl_flags_tbl)));
         if (ret >= 0) {
             fd_trans_register(ret, &target_inotify_trans);
         }
         return ret;
 #endif
-#endif
-#if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
+#if defined(TARGET_NR_inotify_add_watch)
     case TARGET_NR_inotify_add_watch:
         p = lock_user_string(arg2);
-        ret = get_errno(sys_inotify_add_watch(arg1, path(p), arg3));
+        ret = get_errno(inotify_add_watch(arg1, path(p), arg3));
         unlock_user(p, arg2, 0);
         return ret;
 #endif
-#if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
+#if defined(TARGET_NR_inotify_rm_watch)
     case TARGET_NR_inotify_rm_watch:
-        return get_errno(sys_inotify_rm_watch(arg1, arg2));
+        return get_errno(inotify_rm_watch(arg1, arg2));
+#endif
 #endif
 
 #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
-- 
2.34.1



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

* Re: [PATCH] linux-user: Fix inotify on aarch64
  2022-01-26 20:26 [PATCH] linux-user: Fix inotify on aarch64 Paul Brook
@ 2022-01-27 14:08 ` Laurent Vivier
  2022-01-27 14:10 ` Laurent Vivier
  1 sibling, 0 replies; 3+ messages in thread
From: Laurent Vivier @ 2022-01-27 14:08 UTC (permalink / raw)
  To: Paul Brook, qemu-devel

Le 26/01/2022 à 21:26, Paul Brook a écrit :
> The inotify implementation originally called the raw host syscalls.
> Commit 3b3f24add0 changed this to use the glibc wrappers. However ifdefs
> in syscall.c still test for presence of the raw syscalls.
> 
> This causes a problem on e.g. aarch64 hosts which never had the
> inotify_init syscall - it had been obsoleted by inotify_init1 before
> aarch64 was invented! However it does have a perfectly good glibc
> implementation of inotify_wait.
> 
> Fix this by removing all the raw __NR_inotify_* tests, and instead check
> CONFIG_INOTIFY, which already tests for the glibc functionality we use.
> 
> Also remove the now-pointless sys_inotify* wrappers.
> 
> Tested using x86-64 inotifywatch on aarch64 host, and vice-versa
> 
> Signed-off-by: Paul Brook <paul@nowt.org>
> ---
>   linux-user/fd-trans.c |  5 ++---
>   linux-user/syscall.c  | 50 +++++++++----------------------------------
>   2 files changed, 12 insertions(+), 43 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


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

* Re: [PATCH] linux-user: Fix inotify on aarch64
  2022-01-26 20:26 [PATCH] linux-user: Fix inotify on aarch64 Paul Brook
  2022-01-27 14:08 ` Laurent Vivier
@ 2022-01-27 14:10 ` Laurent Vivier
  1 sibling, 0 replies; 3+ messages in thread
From: Laurent Vivier @ 2022-01-27 14:10 UTC (permalink / raw)
  To: Paul Brook, qemu-devel

Le 26/01/2022 à 21:26, Paul Brook a écrit :
> The inotify implementation originally called the raw host syscalls.
> Commit 3b3f24add0 changed this to use the glibc wrappers. However ifdefs
> in syscall.c still test for presence of the raw syscalls.
> 
> This causes a problem on e.g. aarch64 hosts which never had the
> inotify_init syscall - it had been obsoleted by inotify_init1 before
> aarch64 was invented! However it does have a perfectly good glibc
> implementation of inotify_wait.
> 
> Fix this by removing all the raw __NR_inotify_* tests, and instead check
> CONFIG_INOTIFY, which already tests for the glibc functionality we use.
> 
> Also remove the now-pointless sys_inotify* wrappers.
> 
> Tested using x86-64 inotifywatch on aarch64 host, and vice-versa
> 
> Signed-off-by: Paul Brook <paul@nowt.org>
> ---
>   linux-user/fd-trans.c |  5 ++---
>   linux-user/syscall.c  | 50 +++++++++----------------------------------
>   2 files changed, 12 insertions(+), 43 deletions(-)
> 
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 6941089959..30e7b49112 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -1460,9 +1460,8 @@ TargetFdTrans target_eventfd_trans = {
>       .target_to_host_data = swap_data_eventfd,
>   };
>   
> -#if (defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)) || \
> -    (defined(CONFIG_INOTIFY1) && defined(TARGET_NR_inotify_init1) && \
> -     defined(__NR_inotify_init1))
> +#if defined(CONFIG_INOTIFY) && (defined(TARGET_NR_inotify_init) || \
> +        defined(TARGET_NR_inotify_init1))
>   static abi_long host_to_target_data_inotify(void *buf, size_t len)
>   {
>       struct inotify_event *ev;
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 56a3e17183..17cc38fe34 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -272,9 +272,6 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
>   #if defined(__NR_futex_time64)
>   # define __NR_sys_futex_time64 __NR_futex_time64
>   #endif
> -#define __NR_sys_inotify_init __NR_inotify_init
> -#define __NR_sys_inotify_add_watch __NR_inotify_add_watch
> -#define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch
>   #define __NR_sys_statx __NR_statx
>   
>   #if defined(__alpha__) || defined(__x86_64__) || defined(__s390x__)
> @@ -447,33 +444,6 @@ static int sys_renameat2(int oldfd, const char *old,
>   
>   #ifdef CONFIG_INOTIFY
>   #include <sys/inotify.h>
> -
> -#if defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)
> -static int sys_inotify_init(void)
> -{
> -  return (inotify_init());
> -}
> -#endif
> -#if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
> -static int sys_inotify_add_watch(int fd,const char *pathname, int32_t mask)
> -{
> -  return (inotify_add_watch(fd, pathname, mask));
> -}
> -#endif
> -#if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
> -static int sys_inotify_rm_watch(int fd, int32_t wd)
> -{
> -  return (inotify_rm_watch(fd, wd));
> -}
> -#endif
> -#ifdef CONFIG_INOTIFY1
> -#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
> -static int sys_inotify_init1(int flags)
> -{
> -  return (inotify_init1(flags));
> -}
> -#endif
> -#endif
>   #else
>   /* Userspace can usually survive runtime without inotify */
>   #undef TARGET_NR_inotify_init
> @@ -12263,35 +12233,35 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>       case TARGET_NR_futex_time64:
>           return do_futex_time64(cpu, arg1, arg2, arg3, arg4, arg5, arg6);
>   #endif
> -#if defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init)
> +#ifdef CONFIG_INOTIFY
> +#if defined(TARGET_NR_inotify_init)
>       case TARGET_NR_inotify_init:
> -        ret = get_errno(sys_inotify_init());
> +        ret = get_errno(inotify_init());
>           if (ret >= 0) {
>               fd_trans_register(ret, &target_inotify_trans);
>           }
>           return ret;
>   #endif
> -#ifdef CONFIG_INOTIFY1
> -#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
> +#if defined(TARGET_NR_inotify_init1) && defined(CONFIG_INOTIFY1)
>       case TARGET_NR_inotify_init1:
> -        ret = get_errno(sys_inotify_init1(target_to_host_bitmask(arg1,
> +        ret = get_errno(inotify_init1(target_to_host_bitmask(arg1,
>                                             fcntl_flags_tbl)));
>           if (ret >= 0) {
>               fd_trans_register(ret, &target_inotify_trans);
>           }
>           return ret;
>   #endif
> -#endif
> -#if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
> +#if defined(TARGET_NR_inotify_add_watch)
>       case TARGET_NR_inotify_add_watch:
>           p = lock_user_string(arg2);
> -        ret = get_errno(sys_inotify_add_watch(arg1, path(p), arg3));
> +        ret = get_errno(inotify_add_watch(arg1, path(p), arg3));
>           unlock_user(p, arg2, 0);
>           return ret;
>   #endif
> -#if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
> +#if defined(TARGET_NR_inotify_rm_watch)
>       case TARGET_NR_inotify_rm_watch:
> -        return get_errno(sys_inotify_rm_watch(arg1, arg2));
> +        return get_errno(inotify_rm_watch(arg1, arg2));
> +#endif
>   #endif
>   
>   #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)

Applied to my linux-user-for-7.0 branch.

Thanks,
Laurent



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

end of thread, other threads:[~2022-01-27 14:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 20:26 [PATCH] linux-user: Fix inotify on aarch64 Paul Brook
2022-01-27 14:08 ` Laurent Vivier
2022-01-27 14:10 ` Laurent Vivier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.