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

[Warner Losh <imp@bsdimp.com>]

[-- Attachment #1: Type: text/plain, Size: 4344 bytes --]

On Tue, Nov 5, 2024 at 3:49 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> 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?
>

Oh! I understand.... I thought there was a gap above SIGRTMAX. It
sure looks like there is. However, 0177 (127) is used to signal that
the process is STOPPED, so can't be used. This is why SIGRTMAX
is 126 and not 127. There's room in sigset_t, but that's not sufficient.
And it has to be an actual signal we send, not just a flag.

So forget I said anything. This was a silly idea. If we find any real thing
that's using SIGRTMAX, we'll cope.

Warner

[-- Attachment #2: Type: text/html, Size: 6204 bytes --]