Date: Sun, 04 Mar 2001 14:23:03 -0800 (PST) From: John Baldwin <jhb@FreeBSD.org> To: "Justin T. Gibbs" <gibbs@scsiguy.com> Cc: cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_intr.c src/sys/sys interrupt.h Message-ID: <XFMail.010304142303.jhb@FreeBSD.org> In-Reply-To: <200103042151.f24Lp6O83223@aslan.scsiguy.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 04-Mar-01 Justin T. Gibbs wrote: >>> Sure there is. You can block the "releaser" until the interrupt routine >>> exits. >>> There is no other way to resolve the race condition. >> >>And what if the routine that is deregeristering the interrupt handler holds a >>mutex that the ithread is blocked on? We will deadlock if we wait for the >>ithread to exit. > > That is a driver bug or a bug in the deregistering mechanism. There > shouldn't > be a need for that code to hold any locks after it has decided whether to > accept the detach and during the removal of the interrupt handler. I'd rather have the code not care either way to make it easier on driver writers if need be. I do agree that drivers probably shouldn't be holding locks while removing handlers. Note that pccard does make a lot of the cases more extreme as the hardware can leave at any time. >>> Perhaps you should just change the >>> handler's entry point to the removal function and set the ithread runable. >>> The >>> new entry point would remove the handler and wakeup the thread waiting for >>> the >>> entry to be removed. >> >>This _still_ doesn't help if we are already executing the handler. > > Sure it does. Once the new handler is run, you are guaranteed that the old > handler > is not running and can cannot be run again. If the handler is running, the > new handler > will not be run until the next time through the ihandler list. The trick is > making > sure that the ithread loops at least one more time. So, if you sleep until > the > your new handler is run, you are always safe. Erm, did you read what my code does? If I have registerd the foo() function and the ithread blocks while it is executing foo() and while it is blocked another thread removes foo() from the list, what possible good can changing hte functino pointer do to change the fact that the ithread is executing foo()? I guess I could go rewrite the stack or something really arcane, which would break any mutexes foo() might be holding, etc. Changing the function pointer doesn't _fix_ that, and marking the intrhand as dead via IH_DEAD achieves the same result as changing the function pointer. Also, since I set it_need, I _do_ force the ithread to loop again to purge the item from the list. I guess your new change to this is you want to sleep on the intrhand until the ithread runs to release it? I don't see why that is needed. Once the intrhand is dead, we don't dereference the function pointer again, so we aren't in danger of calling the function on a free'd softc, etc. The only bad case is if the function is already running, and changing the function pointer doesn't do anything to help with that. >>It uses a normal TAILQ. Since pointer updates are atomic on all archs we >>support atm and since we fully initialize a struct intrhand before we put it >>on a list so there is currently no problem there. > > How do you avoid the race when two threads attempt to add onto the list at > the same time? A single atomic store doesn't help you unless you can safely > test and recover from a colision. The mutex protects against this. The only unprotected case is the ithread walking the list while another thread is adding an item. > -- > Justin -- John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.010304142303.jhb>