On Thursday 26 April 2007 23:18, Julian Elischer wrote: > Hans Petter Selasky wrote: > > Hi, > > > > In the new USB stack I have defined the following: > > > > u_int32_t > > mtx_drop_recurse(struct mtx *mtx) > > { > > u_int32_t recurse_level = mtx->mtx_recurse; > > u_int32_t recurse_curr = recurse_level; > > > > mtx_assert(mtx, MA_OWNED); > > > > while(recurse_curr--) { > > mtx_unlock(mtx); > > } > > > > return recurse_level; > > } > > The reason that mutexes ever recurse in the first place is usually because > one piece of code calls itself (or a related piece of code) in a blind > manner.. in other words, it doesn't know it is doing so. The whole concept > of recursing mutexes is a bit gross. The concept of blindly unwinding them > is I think just asking for trouble. > > Further the idea that holding a mutex "except for when we sleep" is a > generally bright idea is also a bit odd to me.. If you hold a mutex and > release it during sleep you probably should invalidate all assumptions you > made during the period before you slept as whatever you were protecting has > possibly been raped while you slept. I have seen too many instances where > people just called msleep and dropped the mutex they held, picked it up > again on wakeup, and then blithely continued on without checking what > happened while they were asleep. > > It seems to me that if someone else held that lock for some purpose when > you slept, then you don't know what it was that they were trying to > protect, so you don't know what to recheck when you wake up.. I think this > is extremely dangerous as you don't know when you are in this situation.. > > Personally I think Matt Dillon had a good idea when he suggested that the > need to sleep when holding a lock should require the lock holder to back > out as far as needed as to be able to see why the lock was held and to > comfortably cope with the situaution when the protected structures got > frobbed while it slept. (He actually at one time committed some primitives > to do this. I forget their names and htey were never used, but the idea has > some merrit.) > > This change may be OK in the situations you plan but it makes me very > uncomfortable. it basically should have a big sign on it saying "You could > spend years looking for the wierd bugs you are likely to get if you use > this" on it.
Yes, but could we factor this code out into a msleep_recurse() function then? And the mtx_pickup/mtx_drop functions I would rather see in "kern_synch.c", because it is so much faster to change the recurse level, than it is to do a while loop. --HPS _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"