* [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
@ 2021-01-19 18:24 Alistair Francis
2021-01-20 20:12 ` Andreas K. Hüttel
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alistair Francis @ 2021-01-19 18:24 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, dilfridge
Cc: alistair.francis, bmeng.cn, palmer, alistair23
When mapping the host waitid status to the target status we previously
just used decoding information in the status value. This doesn't follow
what the waitid documentation describes, which instead suggests using
the si_code value for the decoding. This results in the incorrect values
seen when calling waitid. This is especially apparent on RV32 where all
wait calls use waitid (see the bug case).
This patch just passes the waitid status directly back to the guest.
Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
v2:
- Set tinfo->_sifields._sigchld._status directly from status
linux-user/signal.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 73de934c65..7eecec46c4 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -349,8 +349,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
case TARGET_SIGCHLD:
tinfo->_sifields._sigchld._pid = info->si_pid;
tinfo->_sifields._sigchld._uid = info->si_uid;
- tinfo->_sifields._sigchld._status
- = host_to_target_waitstatus(info->si_status);
+ tinfo->_sifields._sigchld._status = info->si_status;
tinfo->_sifields._sigchld._utime = info->si_utime;
tinfo->_sifields._sigchld._stime = info->si_stime;
si_type = QEMU_SI_CHLD;
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
2021-01-19 18:24 [PATCH v2 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
@ 2021-01-20 20:12 ` Andreas K. Hüttel
2021-01-21 15:15 ` Andreas K. Hüttel
2021-02-13 16:08 ` Laurent Vivier
2021-02-13 16:15 ` Laurent Vivier
2 siblings, 1 reply; 6+ messages in thread
From: Andreas K. Hüttel @ 2021-01-20 20:12 UTC (permalink / raw)
To: qemu-devel, qemu-riscv; +Cc: palmer, bmeng.cn, Alistair Francis, alistair23
[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]
>
> This patch just passes the waitid status directly back to the guest.
>
This works at least as well as the previous versions, so ++ from me.
Will do more testing over the next days to see if it maybe also improves the
additional oddities I observed.
Tested-by: Andreas K. Hüttel <dilfridge@gentoo.org>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v2:
> - Set tinfo->_sifields._sigchld._status directly from status
>
> linux-user/signal.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..7eecec46c4 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -349,8 +349,7 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo, case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> + tinfo->_sifields._sigchld._status = info->si_status;
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, qa, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
2021-01-20 20:12 ` Andreas K. Hüttel
@ 2021-01-21 15:15 ` Andreas K. Hüttel
2021-02-12 21:44 ` Alistair Francis
0 siblings, 1 reply; 6+ messages in thread
From: Andreas K. Hüttel @ 2021-01-21 15:15 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Alistair Francis, bmeng.cn, palmer, Andreas K. Hüttel,
alistair23
[-- Attachment #1: Type: text/plain, Size: 1933 bytes --]
Am Mittwoch, 20. Januar 2021, 22:12:30 EET schrieb Andreas K. Hüttel:
> > This patch just passes the waitid status directly back to the guest.
>
> This works at least as well as the previous versions, so ++ from me.
>
> Will do more testing over the next days to see if it maybe also improves the
> additional oddities I observed.
>
So the patch is good since it clearly resolves the linked bug.
However, something else is still broken (maybe related; unchanged compared to
the previous patch version). I keep seeing hanging "qemu-riscv32 /bin/bash
..." processes using 100% cpu. If I attach strace (on the host arch x86-64) to
qemu, I see an infinite loop:
waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
...
Unfortunately I do not have a simple reproducer. This is somewhere in the
middle of our build system...
Otherwise, I can re-build glibc, gcc, binutils, make in the qemu chroot
without obvious problems, with one striking strange detail - "make" refuses to
run more than one concurrent process (even with MAKEOPTS="-j9"). Something is
off there.
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, qa, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
2021-01-21 15:15 ` Andreas K. Hüttel
@ 2021-02-12 21:44 ` Alistair Francis
0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2021-02-12 21:44 UTC (permalink / raw)
To: Andreas K. Hüttel
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Thu, Jan 21, 2021 at 7:15 AM Andreas K. Hüttel <dilfridge@gentoo.org> wrote:
>
> Am Mittwoch, 20. Januar 2021, 22:12:30 EET schrieb Andreas K. Hüttel:
> > > This patch just passes the waitid status directly back to the guest.
> >
> > This works at least as well as the previous versions, so ++ from me.
> >
> > Will do more testing over the next days to see if it maybe also improves the
> > additional oddities I observed.
> >
>
> So the patch is good since it clearly resolves the linked bug.
>
> However, something else is still broken (maybe related; unchanged compared to
> the previous patch version). I keep seeing hanging "qemu-riscv32 /bin/bash
> ..." processes using 100% cpu. If I attach strace (on the host arch x86-64) to
> qemu, I see an infinite loop:
>
> waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> ...
>
> Unfortunately I do not have a simple reproducer. This is somewhere in the
> middle of our build system...
>
> Otherwise, I can re-build glibc, gcc, binutils, make in the qemu chroot
> without obvious problems, with one striking strange detail - "make" refuses to
> run more than one concurrent process (even with MAKEOPTS="-j9"). Something is
> off there.
Ping!
Even though it seems like there are still issues with RV32, this is a
step in the right direction.
Alistair
>
> --
> Andreas K. Hüttel
> dilfridge@gentoo.org
> Gentoo Linux developer
> (council, qa, toolchain, base-system, perl, libreoffice)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
2021-01-19 18:24 [PATCH v2 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-20 20:12 ` Andreas K. Hüttel
@ 2021-02-13 16:08 ` Laurent Vivier
2021-02-13 16:15 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2021-02-13 16:08 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv, dilfridge
Cc: alistair23, bmeng.cn, palmer
Le 19/01/2021 à 19:24, Alistair Francis a écrit :
> When mapping the host waitid status to the target status we previously
> just used decoding information in the status value. This doesn't follow
> what the waitid documentation describes, which instead suggests using
> the si_code value for the decoding. This results in the incorrect values
> seen when calling waitid. This is especially apparent on RV32 where all
> wait calls use waitid (see the bug case).
>
> This patch just passes the waitid status directly back to the guest.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v2:
> - Set tinfo->_sifields._sigchld._status directly from status
>
> linux-user/signal.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..7eecec46c4 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -349,8 +349,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> + tinfo->_sifields._sigchld._status = info->si_status;
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
2021-01-19 18:24 [PATCH v2 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-20 20:12 ` Andreas K. Hüttel
2021-02-13 16:08 ` Laurent Vivier
@ 2021-02-13 16:15 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2021-02-13 16:15 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv, dilfridge
Cc: alistair23, bmeng.cn, palmer
Le 19/01/2021 à 19:24, Alistair Francis a écrit :
> When mapping the host waitid status to the target status we previously
> just used decoding information in the status value. This doesn't follow
> what the waitid documentation describes, which instead suggests using
> the si_code value for the decoding. This results in the incorrect values
> seen when calling waitid. This is especially apparent on RV32 where all
> wait calls use waitid (see the bug case).
>
> This patch just passes the waitid status directly back to the guest.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v2:
> - Set tinfo->_sifields._sigchld._status directly from status
>
> linux-user/signal.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..7eecec46c4 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -349,8 +349,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> + tinfo->_sifields._sigchld._status = info->si_status;
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
>
Applied to my linux-user-for-6.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-13 16:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-19 18:24 [PATCH v2 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-20 20:12 ` Andreas K. Hüttel
2021-01-21 15:15 ` Andreas K. Hüttel
2021-02-12 21:44 ` Alistair Francis
2021-02-13 16:08 ` Laurent Vivier
2021-02-13 16:15 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).