re: port-mips/52892: Tests hang on MIPS

[christos%zoulas.com@localhost (Christos Zoulas)]

The following reply was made to PR port-mips/52892; it has been noted by GNATS.

From: christos%zoulas.com@localhost (Christos Zoulas)
To: matthew green <mrg%eterna.com.au@localhost>
Cc: port-mips-maintainer%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
	netbsd-bugs%netbsd.org@localhost, gson%gson.org@localhost (Andreas Gustafsson), 
	gnats-bugs%NetBSD.org@localhost
Subject: re: port-mips/52892: Tests hang on MIPS
Date: Sun, 22 Apr 2018 17:32:37 -0400

 On Apr 23,  7:11am, mrg%eterna.com.au@localhost (matthew green) wrote:
 -- Subject: re: port-mips/52892: Tests hang on MIPS
 
 | > | >  Index: kern_condvar.c
 | > | >  =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 | =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 | =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 | > | >  RCS file: /cvsroot/src/sys/kern/kern_condvar.c,v
 | > | >  retrieving revision 1.41
 | > | >  diff -u -r1.41 kern_condvar.c
 | > | >  --- kern_condvar.c	30 Jan 2018 07:52:22 -0000	1.41
 | > | >  +++ kern_condvar.c	22 Apr 2018 16:12:19 -0000
 | > | >  @@ -268,9 +268,10 @@
 | > | >  =20
 | > | >   	KASSERT(mutex_owned(mtx));
 | > | >  =20
 | > | >  -	cv_enter(cv, mtx, l);
 | > | >  +	CV_LOCKDEBUG_HANDOFF(l, cv);
 | > | >   	error =3D sleepq_block(0, true);
 | > | >  -	return cv_exit(cv, mtx, l, error);
 | > | >  +	mutex_enter(mtx);
 | > | >  +	return error;
 | > | >   }
 | > |=20
 | > | this can't be right.  you've removed cv_enter() entirely,
 | > | and the cv_exit() only means we don't potentially cv_signal()
 | > | something.  perhaps this is proc_lock being held issue, ie,
 | > | is that the only place it could hang here?
 | >=20
 | > Then cv_wait() is wrong too, because it is the same code; except sleepq_b=
 | lock
 | > is called with false.
 | 
 | what cv_wait() are you looking at?  mine shows:
 | 
 | 235 void
 | 236 cv_wait(kcondvar_t *cv, kmutex_t *mtx)
 | 237 {
 | 238         lwp_t *l =3D curlwp;
 | 239
 | 240         KASSERT(mutex_owned(mtx));
 | 241
 | 242         cv_enter(cv, mtx, l);
 | 243
 | 244         /*
 | 245          * We can't use cv_exit() here since the cv might be destroyed =
 | before
 | 246          * this thread gets a chance to run.  Instead, hand off the loc=
 | kdebug
 | 247          * responsibility to the thread that wakes us up.
 | 248          */
 | 249
 | 250         CV_LOCKDEBUG_HANDOFF(l, cv);
 | 251         (void)sleepq_block(0, false);
 | 252         mutex_enter(mtx);
 | 253 }
 | 
 | note L242.
 | 
 | did you actually test this change? :)
 
 No, it needs the cv_enter...
 
 
 christos