Date: Fri, 24 Sep 2004 22:07:44 +0200 From: Joerg Wunsch <j@uriah.heep.sax.de> To: Nate Lawson <nate@root.org>, John Baldwin <jhb@FreeBSD.org>, Brian Fundakowski Feldman <green@FreeBSD.org>, Scott Long <scottl@FreeBSD.org>, "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/fdc fdc.c fdcvar.h Message-ID: <20040924220744.B96751@uriah.heep.sax.de> In-Reply-To: <4153415C.4070608@root.org>; from nate@root.org on Thu, Sep 23, 2004 at 02:34:20PM -0700 References: <200409231748.19685.jhb@FreeBSD.org> <20040923224739.GE959@green.homeunix.org> <41535A06.7090809@root.org> <200409232112.i8NLCLgQ065917@repoman.freebsd.org> <200409231748.19685.jhb@FreeBSD.org> <20040923224739.GE959@green.homeunix.org> <200409232112.i8NLCLgQ065917@repoman.freebsd.org> <200409231748.19685.jhb@FreeBSD.org> <20040923211225.54E9016A584@hub.freebsd.org> <4153415C.4070608@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Thanks for the interesting discussion. I've learned a bit by that. > > > If a thread is suspended and the module is unloaded, what would > > > be the failure case? > > Leaving unclaimed KVM around that can never be recovered. > It would be really sad if we _designed_ FreeBSD to leak threads. > Windows stopped doing that back with Win98. We're better than that. I wouldn't even have dreamed of leaking resources. ;-) Btw., the main reason for wanting to be able to kldunload the fdc module is to ease development work. It would be very interesting to suspend/resume a thread when the actual implementation changed between the invocations... As M. Warner Losh wrote: > John Baldwin <jhb@freebsd.org> writes: > : 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. [...] > Wouldn't simply unregistering your interrupt before killing the worker > thread handle this case? [...] > The floppy driver, however, doesn't appear to do this. It kills the > thread and then releases the interrupt. It should do something more > like: > + /* Tear down the interrupt before killing the thread */ > + if (fdc->fdc_intr) > + bus_teardown_intr(dev, fdc->res_irq, fdc->fdc_intr); > + fdc->fdc_intr = NULL; > + > /* kill worker thread */ > - fdc->flags |= FDC_KTHREAD_EXIT; > mtx_lock(&fdc->fdc_mtx); > + fdc->flags |= FDC_KTHREAD_EXIT; Sounds reasonable. I'll do it that way. I simply didn't think of the possible interrupt issue. OTOH, the thread will only exit if all bp's are done, which somehow implies that no further interrupts should happen. As M. Warner Losh wrote: > : Why not use kthread_suspend/kthread_suspend_check instead? > because this is safer. The thread exit code will wake up all sleepers > on the thread pointer when the thread is totally dead. This ensures > that the thread is out of the driver and it is safe to proceed. *That* was the information I've been looking for. I already wondered how pccbb (which you've been referring me to) actually woke up the parent. I've just added this piece of information to kthread(9) to spare others the puzzling over it. > However, the fdc->flags |= should be under the lock, because > otherwise you are racing the thread's touching of flags. phk did already most of this. IMHO, it should also be protected inside the worker thread, but since I accidentally forgot to call kthread_exit() anyway, I'll commit these changes in one rush then. -- cheers, J"org .-.-. --... ...-- -.. . DL8DTL http://www.sax.de/~joerg/ NIC: JW11-RIPE Never trust an operating system you don't have sources for. ;-)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040924220744.B96751>