On Mon, 2008-06-09 at 21:52 +0000, Samuel Thibault wrote: > CVSROOT: /cvsroot/hurd > Module name: hurd > Changes by: Samuel Thibault <sthibaul> 08/06/09 21:52:12 > > Modified files: > libdiskfs : ChangeLog dir-lookup.c > > Log message: > 2008-06-06 Samuel Thibault <[EMAIL PROTECTED]> > > * dir-lookup.c (diskfs_S_dir_lookup): Unlock np in case of > errors.
To be clear, as per my previous message, I would like this change reverted and replaced with something less destabilizing. Note that the suggested fix I emailed is not complete: it requires checking to make sure that the relevant code only happens in the right places. (for example, if(newpi) is not exactly kosher since the exit sequence releases a reference without clearing NEWPI. be careful!) But I want to add a further note that Samuel's process--while good--suffers from an important flaw in thinking about locking race conditions. When there is a deadlock, there is certainly a bug. But it is quite incorrect to simply assume that the correct fix is to unlock something before the relevant routine is called. Note that in this case the result is a very dangerous destabilization in the case taht NP = DNP, where Samuel's patch would result in an extra unlock. That could either produce one of those panics that was reported recently (can't remember by whom), or worse yet, unlocking something that should be locked with the result that we get directory corruption errors! The thought process should have been: Oooh, releasing the protid is bad if we are the last reference and holding the lock; indeed, it is not allowed to release the last reference if you hold the lock.... So what should we do? We could (1) release the lock before releasing the protid, or (2), postpone releasing the protid until later. Note that (1) changes the way the locking protocol works for possibly lots of things. Very destabilizing--even if it happened not to cause a bug in a particular case. Option (2) is *guaranteed to be safe*, because there is no harm in holding a reference longer than you have to. Thomas
