Re: [PATCH 4/8] user: Introduce host_interrupt_signal

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Tue, 2024-11-05 at 22:30 +0000, Richard Henderson wrote:
> On 11/5/24 15:50, Ilya Leoshkevich wrote:
> > On Tue, 2024-11-05 at 08:39 -0700, Warner Losh wrote:
> > > On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich
> > > <iii@linux.ibm.com>
> > > wrote:
> > > > Attaching to the gdbstub of a running process requires stopping
> > > > its
> > > > threads. For threads that run on a CPU, cpu_exit() is enough,
> > > > but
> > > > the
> > > > only way to grab attention of a thread that is stuck in a long-
> > > > running
> > > > syscall is to interrupt it with a signal.
> > > > 
> > > > Reserve a host realtime signal for this, just like it's already
> > > > done
> > > > for TARGET_SIGABRT on Linux. This may reduce the number of
> > > > available
> > > > guest realtime signals by one, but this is acceptable, since
> > > > there
> > > > are
> > > > quite a lot of them, and it's unlikely that there are apps that
> > > > need
> > > > them all.
> > > > 
> > > > Set signal_pending for the safe_sycall machinery to prevent
> > > > invoking
> > > > the syscall. This is a lie, since we don't queue a guest
> > > > signal,
> > > > but
> > > > process_pending_signals() can handle the absence of pending
> > > > signals.
> > > > The syscall returns with QEMU_ERESTARTSYS errno, which arranges
> > > > for
> > > > the automatic restart. This is important, because it helps
> > > > avoiding
> > > > disturbing poorly written guests.
> > > > 
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > >   bsd-user/signal.c     | 12 ++++++++++++
> > > >   include/user/signal.h |  2 ++
> > > >   linux-user/signal.c   | 11 +++++++++++
> > > >   3 files changed, 25 insertions(+)
> > > > 
> > > > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> > > > index a2b11a97131..992736df5c5 100644
> > > > --- a/bsd-user/signal.c
> > > > +++ b/bsd-user/signal.c
> > > > @@ -49,6 +49,8 @@ static inline int sas_ss_flags(TaskState *ts,
> > > > unsigned long sp)
> > > >           on_sig_stack(ts, sp) ? SS_ONSTACK : 0;
> > > >   }
> > > > 
> > > > +int host_interrupt_signal = SIGRTMAX;
> > > > +
> > > > 
> > > 
> > > 
> > > I'd be tempted to use SIGRTMAX + 1 or even TARGET_NSIG. 127 or
> > > 128
> > > would
> > > work and not overflow any arrays (or hit any bounds tests) I'd
> > > likely
> > > use SIGRTMAX + 1,
> > > though, since it avoids any edge-cases from sig == NSIG that
> > > might be
> > > in the code
> > > unnoticed.
> > > 
> > > Now, having said that, I don't think that there's too many (any?)
> > > programs we need
> > > to run as bsd-user that have real-time signals, much less one
> > > that
> > > uses SIGRTMAX,
> > > but stranger things have happened. But it is a little wiggle room
> > > just in case.
> > > 
> > > Other than that:
> > > 
> > > Reviewed-by: Warner Losh <imp@bsdimp.com>
> > 
> > Thanks for the suggestion, I'll use SIGRTMAX + 1 in v2.
> 
> 
> That can't be right -- SIGRTMAX+1 is not a valid signal.
> 
> 
> r~

I have to admit I didn't look into this too deeply, but I ran the
following experiment on a FreeBSD 14.1 box:

    /usr/include $ grep -R SIGRTMAX .
    ./sys/signal.h:#define  SIGRTMAX        126

    $ sleep 100 &
    $ kill -126 %1
    [1]   Unknown signal: 126     sleep 100

    $ sleep 100 &
    $ kill -127 %1
    [1] + Unknown signal: 0       sleep 100

Clearly, something is wrong - at least with the shell - but at the
same time the signal delivery seems to have occurred.

Warner, does the above look normal to you?