Date: Thu, 23 Sep 2004 19:23:40 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Nate Lawson <nate@root.org> Cc: Joerg Wunsch <joerg@FreeBSD.org> Subject: Re: cvs commit: src/sys/dev/fdc fdc.c fdcvar.h Message-ID: <200409231923.40285.jhb@FreeBSD.org> In-Reply-To: <41535A06.7090809@root.org> References: <200409232112.i8NLCLgQ065917@repoman.freebsd.org> <20040923224739.GE959@green.homeunix.org> <41535A06.7090809@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 23 September 2004 07:19 pm, Nate Lawson wrote:
> Brian Fundakowski Feldman wrote:
> > On Thu, Sep 23, 2004 at 05:48:19PM -0400, John Baldwin wrote:
> >>On Thursday 23 September 2004 05:12 pm, Joerg Wunsch wrote:
> >>>joerg 2004-09-23 21:12:21 UTC
> >>>
> >>> FreeBSD src repository
> >>>
> >>> Modified files:
> >>> sys/dev/fdc fdc.c fdcvar.h
> >>> Log:
> >>> Implement terminating the worker thread when the driver is about to
> >>> be deregistered.
> >>>
> >>> Not yet tested, since by now, GEOM doesn't want us to deregister. PHK
> >>> wants to fix that RSN.
> >>
> >>Note that relying on a wakeup from your own code is not safe if you
> >> expect fdc to be a module since you could wakeup the thread doing the
> >> kldunload (and thus detach) and then be interrupted for an interrupt and
> >> it could unmap the memory backing that function before you get a chance
> >> to run again resulting in a panic. There is a special wakeup on the
> >> proc pointer in exit1() for kthreads to handle this case. I should
> >> likely move that into kthread_exit() however (which wouldn't be a
> >> functional change as far as you are concerned).
> >
> > For example, in pff(4), ignoring the unrelated lines of code:
> >
> > static void
> > pff_per_device_kthread(void *argp)
> > {
> > mtx_lock(&sc->sc_lock);
> > while (!sck->sck_exit) {
> > if (!sck->sck_update && !sck->sck_reset &&
> > STAILQ_EMPTY(&sck->sck_traps))
> > cv_wait(&sck->sck_needed, &sc->sc_lock);
> > }
> > mtx_unlock(&sc->sc_lock);
> > kthread_exit(0);
> > }
> >
> > static void
> > pff_kill_kthread(struct pff_softc *sc)
> > {
> > if (sc->sc_flags & SC_KTHREAD) {
> > mtx_lock(&sc->sc_lock);
> > sc->sc_kthread.sck_exit = 1;
> > cv_signal(&sc->sc_kthread.sck_needed);
> > msleep(sc->sc_kthread.sck_proc, &sc->sc_lock, PPAUSE | PDROP,
> > "pffktc", 0);
> > }
> > }
>
> Or simpler:
>
> foo_kill():
> error = kthread_suspend(p, kproc_shutdown_wait * hz);
> if (error == EWOULDBLOCK)
> printf("timed out\n");
>
> foo_thread():
> for (;;) {
> mtx_unlock(&bdlock);
> kthread_suspend_check(bufdaemonproc);
> ...
> mtx_lock(&bdlock);
> if (numdirtybuffers <= lodirtybuffers)
> msleep(&bd_request, &bdlock, PVM, "psleep", hz);
That doesn't actually cause the thread to exit, it just goes to sleep. If fdc
wants to support detaching and kldunload it needs the thread to actually go
away, not just go to sleep.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200409231923.40285.jhb>