From owner-cvs-src@FreeBSD.ORG Fri Sep 24 01:19:36 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D884A16A4E7; Fri, 24 Sep 2004 01:19:36 +0000 (GMT) Received: from harmony.village.org (rover.village.org [168.103.84.182]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5ACE243D58; Fri, 24 Sep 2004 01:19:36 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (harmony.village.org [10.0.0.6]) by harmony.village.org (8.13.1/8.13.1) with ESMTP id i8O1Ih78023322; Thu, 23 Sep 2004 19:18:43 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Thu, 23 Sep 2004 19:19:50 -0600 (MDT) Message-Id: <20040923.191950.04529674.imp@bsdimp.com> To: jhb@FreeBSD.org From: "M. Warner Losh" In-Reply-To: <200409231923.40285.jhb@FreeBSD.org> References: <20040923224739.GE959@green.homeunix.org> <41535A06.7090809@root.org> <200409231923.40285.jhb@FreeBSD.org> X-Mailer: Mew version 3.3 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: green@FreeBSD.org cc: src-committers@FreeBSD.org cc: joerg@FreeBSD.org cc: cvs-src@FreeBSD.org cc: cvs-all@FreeBSD.org cc: nate@root.org Subject: Re: cvs commit: src/sys/dev/fdc fdc.c fdcvar.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Sep 2004 01:19:37 -0000 In message: <200409231923.40285.jhb@FreeBSD.org> John Baldwin 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. There is a special wakeup on the : proc pointer in exit1() for kthreads to handle this case. Wouldn't simply unregistering your interrupt before killing the worker thread handle this case? You are sleeping in the detach path then until the worker thread goes away. Once it is gone, it is safe to proceed to detach. cbb does this and I've not had any problems unloading it. In fact, it was how you told me to write cbb a long time ago. Once the bus_teardown_intr() returns, you are guaranteed the interrupt won't get called. 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: Index: fdc.c =================================================================== RCS file: /home/ncvs/src/sys/dev/fdc/fdc.c,v retrieving revision 1.291 diff -u -r1.291 fdc.c --- fdc.c 23 Sep 2004 21:12:21 -0000 1.291 +++ fdc.c 24 Sep 2004 01:16:09 -0000 @@ -1636,9 +1636,14 @@ if ((error = bus_generic_detach(dev))) return (error); + /* 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; while ((fdc->flags & FDC_KTHREAD_ALIVE) != 0) msleep(&fdc->fdc_thread, &fdc->fdc_mtx, PRIBIO, "fdcdet", 0); mtx_unlock(&fdc->fdc_mtx); To resolve the problem (and fix the race on flags I noted earlier). Without the interrtupt firing, then you can't have what you described. : I should : likely move that into kthread_exit() however (which wouldn't be a : functional change as far as you are concerned). I'm not sure I understand this part of your statement. Warner