Date: Sat, 10 Oct 2015 14:31:41 -0500 From: Don Lee <[email protected]>
On Oct 10, 2015, at 10:27 AM, Taylor R Campbell <[email protected]> wrote: > proc_lock may be held by another thread even if the caller is > guaranteed not to hold it. The other thread may furthermore be > waiting on p->p_lock, in which case acquiring p->p_lock here would > lead to deadlock -- but mutex_tryenter would simply fail. > > The optimization is to optimistically assume nobody else is holding > proc_lock and do mutex_tryenter. Otherwise, we back out and acquire > both locks in the correct order. Pardon me for opining on this without full knowledge, but it seems a bit creepy to me to be dealing with locks as though you don't know who holds them. The normal case should be that either you hold the lock or you don't. You should know. If you don't, there is a hierarchy and an order to claiming them, and you follow it. There is a lock order: proc_lock first, then p->p_lock for any single p. Nobody is proposing to violate or weasel around this lock order. The patch does not reverse the lock order, nor allow the caller to hold or not hold proc_lock -- the caller must not hold proc_lock. The patch only transitions from holding only p->p_lock to holding both proc_lock and p->p_lock both. Releasing p->p_lock here is OK because this branch is about to release all locks and sleep anyway, so subsequent code does not rely on any invariants that are lost. The transition could be done with mutex_exit(p->p_lock); mutex_enter(proc_lock); mutex_enter(p->p_lock); but if usually no other threads hold proc_lock, it costs less on average to do if (!mutex_tryenter(proc_lock)) { mutex_exit(p->p_lock); mutex_enter(proc_lock); mutex_enter(p->p_lock); } instead, since each mutex_(try)enter entails an expensive atomic operation.
