Re: Anomalies while handling p_nstopchild count

[Don Lee <MacPPC2%c.icompute.com@localhost>]

On Oct 10, 2015, at 10:27 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:

>   Date: Sat, 10 Oct 2015 11:15:20 -0400
>   From: christos%zoulas.com@localhost (Christos Zoulas)
> 
>   On Oct 10,  2:37pm, campbell+netbsd-tech-kern%mumble.net@localhost (Taylor R Campbell) wrote:
>   -- Subject: Re: Anomalies while handling p_nstopchild count
> 
>   | I don't think that's why he put that there -- it's just that the lock
>   | order is proc_lock first, then p->p_lock for any p.  This is the
>   | optimistic reverse-order locking fragment that the mutex(9) man page
>   | suggests.
> 
>   Yes, but if you know proc_lock is not being held, calling tryenter is
>   not an optimization! The man page should be clarified that the example
>   checks and corrects the lock order *if it was broken*. Unless I am missing
>   something obvious...
> 
> 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.

It should be the very rare case that you don't know who holds what.
Even the case where a lock is to guarantee the order of operation on
other locks, or to ensure that some other set of locks gets set/released
atomically should be viewed with suspicion. Optimizations require even
greater suspicion.

We should know a proper order and hierarchy, and follow it. Any time we
"optimistically" ignore or grab a lock, we are creating a situation that is likely
to work, rather one that is known to be correct. It begs for race conditions
and deadlocks, which we know are lots of fun to debug. ;->

Just my opinion.

-dgl-