Chris Kirby wrote: > Matthew Ahrens wrote: >> 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(). > > The problem in this case turns out to be a bit harder. > dsl_dataset_open_obj can be called with dp_config_rwlock held for > read or write, or not held at all. An obvious way to fix this would > be to have the callers pass an arg saying whether or not the lock is > held, but that's esp. ugly in this case since there are ~30 callers, > and at least some of them probably don't grab the lock directly. > Its actually OK if we re-grab a read lock when we already hold it. This will just cause the readers count to increment in the lock, and we will drop it twice, so the count will be correct in the end. But we don't want to try to obtain a read lock if we already have a write lock. So it works to just use 'RW_WRITE_HELD()' here.
-Mark
