On Thu, Nov 11, 2021 at 02:13:26PM -0600, Scott Cheloha wrote:
> On Thu, Nov 11, 2021 at 08:53:20PM +0100, Mark Kettenis wrote:
> > > Date: Thu, 11 Nov 2021 13:30:04 -0600
> > > From: Scott Cheloha <scottchel...@gmail.com>
> > > 
> > > My understanding of sigsuspend(2) is that it only returns if a signal
> > > is delivered to the calling thread.  However, in sys_sigsuspend() we
> > > pass &p->p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9).
> > > 
> > > Are we actually waiting for a wakeup on that channel?  Or can we sleep
> > > on &nowake here?  Patch attached.  Note that we don't need to loop
> > > here anymore: we can't receieve wakeup so tsleep_nsec(9) will never
> > > return zero.
> > 
> > I don't think we expect a wakeup on that channel.  But the loop is
> > still needed as a spurious wakeup may happen.
> 
> I'm not quite sure how you'd get a spurious wakeup in this context,
> but I'll take your word for it.
> 
> So use &nowake to indicate we're not expecting a wakeup, but keep the
> loop.
> 
> Index: kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 kern_sig.c
> --- kern_sig.c        24 Oct 2021 00:02:25 -0000      1.287
> +++ kern_sig.c        11 Nov 2021 20:10:51 -0000
> @@ -509,7 +509,7 @@ dosigsuspend(struct proc *p, sigset_t ne
>  }
>  
>  /*
> - * Suspend process until signal, providing mask to be set
> + * Suspend thread until signal, providing mask to be set
>   * in the meantime.  Note nonstandard calling convention:
>   * libc stub passes mask, not pointer, to save a copyin.
>   */
> @@ -519,12 +519,10 @@ sys_sigsuspend(struct proc *p, void *v, 
>       struct sys_sigsuspend_args /* {
>               syscallarg(int) mask;
>       } */ *uap = v;
> -     struct process *pr = p->p_p;
> -     struct sigacts *ps = pr->ps_sigacts;
>  
>       dosigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
> -     while (tsleep_nsec(ps, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
> -             /* void */;
> +     while (tsleep_nsec(&nowake, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
> +             continue;
>       /* always return EINTR rather than ERESTART... */
>       return (EINTR);
>  }
> 

OK claudio@
-- 
:wq Claudio

Reply via email to