svn commit: r346507 - in projects/fuse2/sys: kern sys

[Alan Somers]

On Tue, Apr 23, 2019 at 6:47 AM Konstantin Belousov <kostikbel at gmail.com> wrote:
>
> On Mon, Apr 22, 2019 at 11:27:54AM -0600, Alan Somers wrote:
> > On Mon, Apr 22, 2019 at 11:10 AM Konstantin Belousov
> > <kostikbel at gmail.com> wrote:
> > >
> > > On Sun, Apr 21, 2019 at 11:04:06PM +0000, Alan Somers wrote:
> > > > Author: asomers
> > > > Date: Sun Apr 21 23:04:06 2019
> > > > New Revision: 346507
> > > > URL: https://svnweb.freebsd.org/changeset/base/346507
> > > >
> > > > Log:
> > > >   fusefs: commit missing files from r346387
> > > >
> > > >   PR:         346357
> > > >   Sponsored by:       The FreeBSD Foundation
> > > >
> > > > Modified:
> > > >   projects/fuse2/sys/kern/kern_sig.c
> > > >   projects/fuse2/sys/sys/signalvar.h
> > > >
> > > > Modified: projects/fuse2/sys/kern/kern_sig.c
> > > > ==============================================================================
> > > > --- projects/fuse2/sys/kern/kern_sig.c        Sun Apr 21 22:53:51 2019        (r346506)
> > > > +++ projects/fuse2/sys/kern/kern_sig.c        Sun Apr 21 23:04:06 2019        (r346507)
> > > > @@ -929,6 +929,23 @@ osigreturn(struct thread *td, struct osigreturn_args *
> > > >  #endif
> > > >  #endif /* COMPAT_43 */
> > > >
> > > > +/* Will this signal be fatal to the current process ? */
> > > > +bool
> > > > +sig_isfatal(struct proc *p, int sig)
> > > > +{
> > > > +     intptr_t act;
> > > > +
> > > > +     act = (intptr_t)p->p_sigacts->ps_sigact[_SIG_IDX(sig)];
> > > > +     if ((intptr_t)SIG_DFL == act) {
> > > > +             int prop;
> > > This is against style.
> > >
> > > > +
> > > > +             prop = sigprop(sig);
> > > > +             return (0 != (prop & (SIGPROP_KILL | SIGPROP_CORE)));
> > > > +     } else {
> > > > +             return (false);
> > > > +     }
> > > > +}
> > > Either your function lacks asserts about the owned locks, or it is racy.
> >
> > Good point.  I'll add lock assertions.
> >
> > >
> > > Said that, is the comment above describes the intent ? The
> > > implementation is too naive. Just for example, blocked signals with
> > > default disposition do not result in the termination. On the other hand,
> > > blocked ignored traps cause immediate termination.
> >
> > I'm using this in a context where the signal has already been
> > delivered and caught.  So it can't be blocked, and it can't be a trap.
> >
> > >
> > > Overall, I do not believe that it is possible to implement that without
> > > duplicating the code of tdsendsignal() and trapsignal(), i.e. you should
> > > additionally provide the originating context, besides a signal number.
> >
> > Do you still believe that even though it doesn't need to consider
> > blocked signals and traps?
> Generally yes, but lets see the specifics of the use.
>
> >
> > >
> > > What you are trying to do there ?
> >
> > It's in a situation where a syscall can't simply return EINTR or
> > ERESTART.  I need to do some extra work to interrupt the syscall (ask
> > the FUSE daemon to interrupt the associated FUSE operation).  If the
> > signal will be fatal, then there's no point in waiting for the FUSE
> > daemon to reply and I can simply return EINTR.  However, if the signal
> > is not fatal, then I need to wait to see if the FUSE daemon to
> > acknowledge the interrupt or else complete the operation like normal.
> In what context does the interruption happen ?  Is it for a thread of the
> fuse daemon, or normal process ?

It's usually a normal process, but it could be another kernel thread,
like aiod.  It will never be the fuse daemon.

>
> Can you point out the specific fragment of code where the function is used ?

Here's the function that uses it:
https://svnweb.freebsd.org/base/projects/fuse2/sys/fs/fuse/fuse_ipc.c?view=markup#l429

-Alan