Re: Anomalies while handling p_nstopchild count

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Mon, 12 Oct 2015 22:25:12 +0000
    From:        Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>
    Message-ID:  <20151012222359.E6F9F609F0%jupiter.mumble.net@localhost>

  | It is not clear to me why it was needed in the first place,

The code is (pre fix) ...

                p->p_waited = 0;
                membar_producer();
                p->p_stat = SSTOP;

My assumption is that the objective was to ensure that no-one would
ever see p_stat == SSTOP; before the p_waited = 0 is also visible.

That can be important, as p_lock (nor any other relevant lock) is not
acquired before inspecting p_stat (see for example find_stopped_child())
so there's nothing preventing another CPU from racing against this pair of
assignments, which without a memory barrier of some kind could be
reordered by the hardware, and detecting p_stat == SSTOP and p_waited != 0.

I don't believe that membar_consumer() is relevant at all, no-one
(for this purpose) cares what order reads from memory actually happen.

Paul's question really amounted to, I think, whether either the fact that
proc_lock will be held while p_waited is set to 0 would make a difference
(I can't see why that would really matter) or more likely, since the code
will become

 		p->p_waited = 0;
		p->p_pptr->p_nstopchild++;
		mutex_exit(proc_lock);
 		membar_producer();
 		p->p_stat = SSTOP;

whether the mutex_exit() makes the membar_producer() redundant.   That
I suspect is true - that is, I can't see how mutexes coould possibly be
expected to work without memory consistency guarantees, so even though
mutex(9) doesn't say it will, I assume that the mutex_xxx() operations
(well, the ones that change things) will do any required memory barriers.

kre