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.
Interesting thread, I know I've seen problems in this area in my ZFS crypto work for exactly this lock. -- Darren J Moffat
