> Date: Mon, 11 May 2015 16:54:54 +0200 > From: Mike Belopuhov <m...@belopuhov.com> > > On Fri, May 08, 2015 at 20:28 +0200, Mark Kettenis wrote: > > > Date: Fri, 8 May 2015 20:15:58 +0200 > > > From: Mike Belopuhov <m...@belopuhov.com> > > > > > > On Fri, May 08, 2015 at 12:34 +0200, Mike Belopuhov wrote: > > > > > I think tsleep(9) and msleep(9) need to release and re-acquire the > > > > > kernel lock in the "cold || panicstr" case. > > > > > > > > Well, it's not hard to do really, but... > > > > > > > > > We might need this for > > > > > handling interrupts during autoconf as soon as we start distributing > > > > > interrupts over CPUs. > > > > > > > > > > > > > ...cold might mean that interrupts are not ready yet. So then we might > > > > need another flag for shutdown? > > > > > > This is what I have come up with. Chunks were taken directly from > > > mi_switch and it seems to do the job just fine. Right now I'm not > > > using any additional flags and it seems to work here. I'll resume > > > testing on Monday, but it looks fairly complete. Any comments? > > > > Makes sense to me. The msleep/tsleep code could be simplified to: > > > > if (__mp_lock_held(&kernel_lock)) { > > hold_count = __mp_release_all(&kernel_lock); > > __mp_acquire_count(&kernel_lock, hold_count); > > } > > > > Indeed. > > > I'm also wondering whether we should change __mp_release_all() to > > simple return 0 if the current CPU does not hold the lock, such that > > the __mp_lock_held() check isn't needed anymore. But that's a > > separate issue. > > > > It would have made the __mp_release_all safer as well since it > wouldn't require an external check. > > I don't have any firther input on this, diff works fine here. > > OK?
ok kettenis@ > diff --git sys/kern/kern_synch.c sys/kern/kern_synch.c > index 03308b4..6f573fc 100644 > --- sys/kern/kern_synch.c > +++ sys/kern/kern_synch.c > @@ -103,10 +103,13 @@ extern int safepri; > int > tsleep(const volatile void *ident, int priority, const char *wmesg, int timo) > { > struct sleep_state sls; > int error, error1; > +#ifdef MULTIPROCESSOR > + int hold_count; > +#endif > > KASSERT((priority & ~(PRIMASK | PCATCH)) == 0); > > #ifdef MULTIPROCESSOR > KASSERT(timo || __mp_lock_held(&kernel_lock)); > @@ -120,10 +123,16 @@ tsleep(const volatile void *ident, int priority, const > char *wmesg, int timo) > * don't run any other procs or panic below, > * in case this is the idle process and already asleep. > */ > s = splhigh(); > splx(safepri); > +#ifdef MULTIPROCESSOR > + if (__mp_lock_held(&kernel_lock)) { > + hold_count = __mp_release_all(&kernel_lock); > + __mp_acquire_count(&kernel_lock, hold_count); > + } > +#endif > splx(s); > return (0); > } > > sleep_setup(&sls, ident, priority, wmesg); > @@ -149,10 +158,13 @@ int > msleep(const volatile void *ident, struct mutex *mtx, int priority, > const char *wmesg, int timo) > { > struct sleep_state sls; > int error, error1, spl; > +#ifdef MULTIPROCESSOR > + int hold_count; > +#endif > > KASSERT((priority & ~(PRIMASK | PCATCH | PNORELOCK)) == 0); > KASSERT(mtx != NULL); > > if (cold || panicstr) { > @@ -163,10 +175,16 @@ msleep(const volatile void *ident, struct mutex *mtx, > int priority, > * in case this is the idle process and already asleep. > */ > spl = MUTEX_OLDIPL(mtx); > MUTEX_OLDIPL(mtx) = safepri; > mtx_leave(mtx); > +#ifdef MULTIPROCESSOR > + if (__mp_lock_held(&kernel_lock)) { > + hold_count = __mp_release_all(&kernel_lock); > + __mp_acquire_count(&kernel_lock, hold_count); > + } > +#endif > if ((priority & PNORELOCK) == 0) { > mtx_enter(mtx); > MUTEX_OLDIPL(mtx) = spl; > } else > splx(spl); > diff --git sys/kern/vfs_subr.c sys/kern/vfs_subr.c > index a26fbe2..a373789 100644 > --- sys/kern/vfs_subr.c > +++ sys/kern/vfs_subr.c > @@ -1664,10 +1664,13 @@ int > vfs_syncwait(int verbose) > { > struct buf *bp; > int iter, nbusy, dcount, s; > struct proc *p; > +#ifdef MULTIPROCESSOR > + int hold_count; > +#endif > > p = curproc? curproc : &proc0; > sys_sync(p, (void *)0, (register_t *)0); > > /* Wait for sync to finish. */ > @@ -1698,11 +1701,21 @@ vfs_syncwait(int verbose) > } > if (nbusy == 0) > break; > if (verbose) > printf("%d ", nbusy); > +#ifdef MULTIPROCESSOR > + if (__mp_lock_held(&kernel_lock)) > + hold_count = __mp_release_all(&kernel_lock); > + else > + hold_count = 0; > +#endif > DELAY(40000 * iter); > +#ifdef MULTIPROCESSOR > + if (hold_count) > + __mp_acquire_count(&kernel_lock, hold_count); > +#endif > } > > return nbusy; > } > >