Neil Perrin wrote: > Matthew Ahrens wrote: >> Neil Perrin wrote: >>> >>> Ricardo M. Correia wrote: >>>> Ricardo M. Correia wrote: >>>>> boolean_t need_lock = !RW_LOCK_HELD(&dp->dp_config_rwlock); >>>>> >>>>> if (need_lock) >>>>> rw_enter(&dp->dp_config_rwlock, RW_READER); >>>>> >>>> Maybe I posted to soon. >>>> Am I right that this specific code (in dsl_dataset.c) would work >>>> correctly if RW_LOCK_HELD() returned true if *another* thread is >>>> holding the lock as a reader? >>> >>> Yes this looks like a bug to me as well. >> >> This is not a bug, it is the designed behavior. To tell if this >> specific thread is holding the lock for reader would require a >> performance hit (eg, see rrwlock.c behavior when rr_writer_wanted is >> set). >> >> --matt > > - I'm confused then. The dp_config_rwlock lock must be held as it's > asserted in dsl_prop_get_ds_locked() However, the lock can be > dropped immediately after the test dsl_dataset_open_obj() as it only > checks if anyone is holding it - not just the caller. Am I missing > something?
So, we use RW_LOCK_HELD() to mean, "might this thread hold the lock?" and it is generally only used in assertions. Eg, some routine should only be called with the lock held, so we "ASSERT(RW_LOCK_HELD(lock))". The fact that sometimes the implementation of RW_LOCK_HELD() returns TRUE when this thread does not in fact hold the lock is fine. In this case, we are using it a bit differently. The current thread must *not* hold the lock for reader. We use RW_LOCK_HELD() to determine definitively if this thread holds the lock for writer or not. The behavior we want is: if this thread already holds the lock for writer, we don't need to re-grab it. Otherwise, we grab the lock for READER. However, now that I go look at the implementation of RW_LOCK_HELD(), it doesn't do what this code expects; it should be using RW_WRITE_HELD(). --matt
