[PATCH] util/compatfd.c: use libc signalfd wrapper instead of raw syscall

Kacper Słomiński posted 1 patch 2 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210905011621.200785-1-kacper.slominski72@gmail.com
meson.build     | 7 +++----
util/compatfd.c | 5 ++---
2 files changed, 5 insertions(+), 7 deletions(-)
[PATCH] util/compatfd.c: use libc signalfd wrapper instead of raw syscall
Posted by Kacper Słomiński 2 years, 7 months ago
This allows the use of native signalfd instead of the sigtimedwait
based emulation on systems other than Linux.

Signed-off-by: Kacper Słomiński <kacper.slominski72@gmail.com>
---
Apologies if I CC'd the wrong maintaineers, it's my first time
submitting patches to QEMU. According to get_maintainers.pl this
file has no maintainers, and the only system supported upstream
that supports signalfd natively is Linux.

Glibc has had the signalfd wrapper since version 2.8 (2008), and
musl has had it since at least version 0.5.0 (2011), and as such
I think using syscall directly is not necessary here.

Found this while porting QEMU to Managarm
(https://github.com/managarm/managarm).

 meson.build     | 7 +++----
 util/compatfd.c | 5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index bf63784812..bcdfea5492 100644
--- a/meson.build
+++ b/meson.build
@@ -1415,10 +1415,9 @@ config_host_data.set('CONFIG_POSIX_MADVISE', cc.links(gnu_source_prefix + '''
   #include <stddef.h>
   int main(void) { return posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); }'''))
 config_host_data.set('CONFIG_SIGNALFD', cc.links(gnu_source_prefix + '''
-  #include <unistd.h>
-  #include <sys/syscall.h>
-  #include <signal.h>
-  int main(void) { return syscall(SYS_signalfd, -1, NULL, _NSIG / 8); }'''))
+  #include <sys/signalfd.h>
+  #include <stddef.h>
+  int main(void) { return signalfd(-1, NULL, SFD_CLOEXEC); }'''))
 config_host_data.set('CONFIG_SPLICE', cc.links(gnu_source_prefix + '''
   #include <unistd.h>
   #include <fcntl.h>
diff --git a/util/compatfd.c b/util/compatfd.c
index a8ec525c6c..ab810c42a9 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -17,7 +17,7 @@
 #include "qemu/thread.h"
 
 #if defined(CONFIG_SIGNALFD)
-#include <sys/syscall.h>
+#include <sys/signalfd.h>
 #endif
 
 struct sigfd_compat_info {
@@ -96,9 +96,8 @@ int qemu_signalfd(const sigset_t *mask)
 #if defined(CONFIG_SIGNALFD)
     int ret;
 
-    ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
+    ret = signalfd(-1, mask, SFD_CLOEXEC);
     if (ret != -1) {
-        qemu_set_cloexec(ret);
         return ret;
     }
 #endif
-- 
2.33.0


Re: [PATCH] util/compatfd.c: use libc signalfd wrapper instead of raw syscall
Posted by Paolo Bonzini 2 years, 7 months ago
On 05/09/21 03:16, Kacper Słomiński wrote:
> This allows the use of native signalfd instead of the sigtimedwait
> based emulation on systems other than Linux.
> 
> Signed-off-by: Kacper Słomiński <kacper.slominski72@gmail.com>
> ---
> Apologies if I CC'd the wrong maintaineers, it's my first time
> submitting patches to QEMU. According to get_maintainers.pl this
> file has no maintainers, and the only system supported upstream
> that supports signalfd natively is Linux.
> 
> Glibc has had the signalfd wrapper since version 2.8 (2008), and
> musl has had it since at least version 0.5.0 (2011), and as such
> I think using syscall directly is not necessary here.
> 
> Found this while porting QEMU to Managarm
> (https://github.com/managarm/managarm).
> 
>   meson.build     | 7 +++----
>   util/compatfd.c | 5 ++---
>   2 files changed, 5 insertions(+), 7 deletions(-)

Queued, thanks!

Paolo

> diff --git a/meson.build b/meson.build
> index bf63784812..bcdfea5492 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1415,10 +1415,9 @@ config_host_data.set('CONFIG_POSIX_MADVISE', cc.links(gnu_source_prefix + '''
>     #include <stddef.h>
>     int main(void) { return posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); }'''))
>   config_host_data.set('CONFIG_SIGNALFD', cc.links(gnu_source_prefix + '''
> -  #include <unistd.h>
> -  #include <sys/syscall.h>
> -  #include <signal.h>
> -  int main(void) { return syscall(SYS_signalfd, -1, NULL, _NSIG / 8); }'''))
> +  #include <sys/signalfd.h>
> +  #include <stddef.h>
> +  int main(void) { return signalfd(-1, NULL, SFD_CLOEXEC); }'''))
>   config_host_data.set('CONFIG_SPLICE', cc.links(gnu_source_prefix + '''
>     #include <unistd.h>
>     #include <fcntl.h>
> diff --git a/util/compatfd.c b/util/compatfd.c
> index a8ec525c6c..ab810c42a9 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -17,7 +17,7 @@
>   #include "qemu/thread.h"
>   
>   #if defined(CONFIG_SIGNALFD)
> -#include <sys/syscall.h>
> +#include <sys/signalfd.h>
>   #endif
>   
>   struct sigfd_compat_info {
> @@ -96,9 +96,8 @@ int qemu_signalfd(const sigset_t *mask)
>   #if defined(CONFIG_SIGNALFD)
>       int ret;
>   
> -    ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
> +    ret = signalfd(-1, mask, SFD_CLOEXEC);
>       if (ret != -1) {
> -        qemu_set_cloexec(ret);
>           return ret;
>       }
>   #endif
>