On Thu, Nov 18, 2021 at 12:30:30PM +0100, Martin Pieuchot wrote:
> On 17/11/21(Wed) 09:51, Scott Cheloha wrote:
> > > On Nov 17, 2021, at 03:22, Martin Pieuchot <m...@openbsd.org> wrote:
> > >
> > > ???On 16/11/21(Tue) 13:55, Visa Hankala wrote:
> > >> Currently, dopselect() and doppoll() call tsleep_nsec() without retry.
> > >> cheloha@ asked if the functions should handle spurious wakeups. I guess
> > >> such wakeups are unlikely with the nowake wait channel, but I am not
> > >> sure if that is a safe guess.
> > >
> > > I'm not sure to understand, are we afraid a thread sleeping on `nowake'
> > > can be awaken? Is it the assumption here?
> >
> > Yes, but I don't know how.
>
> Then I'd suggest we start with understanding how this can happen otherwise
> I fear we are adding more complexity for reasons we don't understands.
>
> > kettenis@ said spurious wakeups were
> > possible on a similar loop in sigsuspend(2)
> > so I mentioned this to visa@ off-list.
>
> I don't understand how this can happen.
>
> > If we added an assert to panic in wakeup(9)
> > if the channel is &nowake, would that be
> > sufficient?
>
> I guess so.
So, something like the attached patch? All variants of wakeup(9) end
up in wakeup_proc(), right?
Wondering if it'd be better (and cheaper) to do the assert at the top
of wakeup_n(9)...
kettenis: Can you explain how a spurious wakeup would actually happen
here or in sigsuspend(2)?
Index: kern_synch.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.180
diff -u -p -r1.180 kern_synch.c
--- kern_synch.c 7 Oct 2021 08:51:00 -0000 1.180
+++ kern_synch.c 19 Nov 2021 01:41:21 -0000
@@ -493,6 +493,8 @@ wakeup_proc(struct proc *p, const volati
{
int s, awakened = 0;
+ KASSERT(chan != &nowake);
+
SCHED_LOCK(s);
if (p->p_wchan != NULL &&
((chan == NULL) || (p->p_wchan == chan))) {
