On Thu, 21 Jun 2007 14:53:20 +0100
Andrew Doran <ad@netbsd.org> wrote:

> 2. In sme_make_dictionary(), why acquire and release the lock so many
> times? Wouldn't it be clearer to acquire it fewer times?
> 
> 3. sme_event_unregister():
> 
> 	if (LIST_EMPTY(&sme_events_list)) {
> 		mutex_exit(&sme_event_mtx);
> 		callout_stop(&seeco);
> 		workqueue_destroy(seewq);
> 
> 		mutex_enter(&sme_event_mtx);
> 		sme_events_initialized = false;
> 
> When you unlock to destroy the work queue / callout, it's possible for
> another thread to do sme_events_init() at the same time, right?
> 
> The problem with holding a lock around those is that later on when
> interrupt handlers have thread context, you may want to acquire the lock
> from an interrupt handler or callout. Those cannot be delayed for long.
> Functions like workqueue_create() may want to sleep long term for memory.
> One solution may be to have two locks. One short term lock for data
> (sme_event_mtx) and one longer term lock for sme_events_initialized
> (sme_event_init_mtx) that you hold while setting up or destroying the
> resources.

Can you please check: http://www.netbsd.org/~xtraeme/envsys2_api3.diff

I added another mutex to protect the operations with callout/workqueue
as you suggested and I removed many mutex_enter/mutex_exit with a few
ones in sme_make_dictionary().

Thanks!
-- 
Juan Romero Pardines	- The NetBSD Project
http://plog.xtrarom.org	- NetBSD/pkgsrc news in Spanish