Re: CVS commit: src/sys

[Andrew Doran <ad%netbsd.org@localhost>]

On Thu, Oct 12, 2023 at 11:55:46AM +0200, J. Hannken-Illjes wrote:
> > On 10. Oct 2023, at 20:58, Andrew Doran <ad%netbsd.org@localhost> wrote:
> > 
> > On Tue, Oct 10, 2023 at 06:00:57PM +0200, J. Hannken-Illjes wrote:
> > 
> >>> cvs rdiff -u -r1.63 -r1.64 src/sys/kern/sys_select.c
> >> 
> >> -    sleepq_unsleep(l, false);
> >> +    sleepq_remove(l->l_sleepq, l, true);
> >>    }
> >>   }
> >>   mutex_spin_exit(lock);
> >> 
> >> Looks like sleepq_remove() unlocks l->l_mutex == lock and
> >> then mutex_spin_exit(lock) will unlock an unlocked mutex.
> > 
> > lock is held before the call to sleepq_remove() and this is also true at the
> > time: l->l_mutex == lock.
> > 
> > After the call lock is still held, but now l->l_mutex != lock, because l has
> > changed state (e.g LSSLEEP -> LSRUN) which causes l_mutex to change in
> > concert.  There is a rough overview here:
> > 
> > https://nxr.netbsd.org/xref/src/sys/kern/kern_lwp.c#156
> > 
> > Did you encounter a problem?
> 
> This is not true for RUMP.  Hero you added sleepq_remove() as
> "sleepq_unsleep(l, true)".  This will unlock l_mutex without changing.
> 
> Just poking around and using sleepq_unsleep(l, false) here makes the
> NFS tests using rump_server pass.

Ah, now I see, thank you.  I committed a fix and will do a test run once my
build completes.

Cheers,
Andrew