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

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

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