Date: Tue, 14 Nov 2000 22:37:08 -0800 From: Jake Burkholder <jburkhol@home.com> To: John Baldwin <jhb@FreeBSD.org> Cc: Mike Smith <msmith@FreeBSD.org>, cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org, jake@io.yi.org Subject: Re: cvs commit: src/sys/sys eventhandler.h Message-ID: <20001115063708.614A0BA7A@io.yi.org> In-Reply-To: Message from John Baldwin <jhb@FreeBSD.org> of "Tue, 14 Nov 2000 20:55:38 PST." <200011150455.UAA12866@john.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
> > On 15-Nov-00 Mike Smith wrote: > >> jhb 2000/11/14 10:23:00 PST > >> > >> Modified files: > >> sys/sys eventhandler.h > >> Log: > >> Only hold the mutex for an eventhandler list while the list is being > >> accessed. > >> Specifically, don't hold the lock while calling event handlers as a > >> handler > >> may tsleep() while holding the mutex. > > > > This is wrong and should be backed out; the tsleep() in the handler is > > the bug. You can't release the lock on the list while you're traversing > > it, since the traversal is holding private state which has to remain > > consistent with the list. > > > > If you want to release the lock on the list, you would have to detect > > when the list is changed and rescan it to find the 'new' current > > location. Only that doesn't work either, since the list traversal allows > > you to remove the current handler from the list. > > > > Please revert this change and fix eventhandler clients so that they don't > > sleep. > > Hrm, I guess I could go hard-code all the shutdown events to not use tsleep but > to use delay()'s. This means hacking up or duplicating things like > suspend_kproc(). :(. You shouldn't be holding a lock while you are calling > functions that aren't related to the resource you are holding. Grrr. Would > it be feasible to say that an eventhandler may not modify a list it is on? > Perhaps extending the eventhandler interface to allow for once-only events > would be sufficient. > The timeout code deals with a similar situation. It saves the next element in the list in a global variable, nextsoftcheck, spls down, calls the timeout handler, spls back up and uses nextsoftcheck as the next element. If untimeout (callout_stop) finds that nextsoftcheck points to the timeout that's about to be deleted, it advances nextsoftcheck to point to the next element. Wouldn't something similar work? Save the next element in the eventhandler_list structure, and then have deregister check if its the same one that's being deleted, and if so advance it along. Holding the mutex over the whole traversal also means you rely on mutex recursion in order to be able to deregister from an event handler. Jake > -- > > 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?20001115063708.614A0BA7A>