Re: [PATCH][BZ #13065] New pthread_barrier algorithm to fulfill barrier destruction requirements.

[Torvald Riegel <triegel at redhat dot com>]

On Fri, 2015-12-18 at 17:47 -0600, Paul E. Murphy wrote:
> 
> On 12/18/2015 10:42 AM, Torvald Riegel wrote:
> > On Sun, 2015-07-19 at 23:52 +0200, Torvald Riegel wrote:
> >> The previous barrier implementation did not fulfill the POSIX
> >> requirements for when a barrier can be destroyed.  Specifically, it was
> >> possible that threads that haven't noticed yet that their round is
> >> complete still access the barrier's memory, and that those accesses can
> >> happen after the barrier has been legally destroyed.
> >> The new algorithm does not have this issue, and it avoids using a lock
> >> internally.
> >>
> 
> > Ping.  Attached is a rebased patch.
> 
> +   We count the number of threads that have entered (IN).  Each thread
> +   increments IN when entering, thus getting a position in the sequence of
> +   threads that are or have been waiting (starting with 1, so we the position
> +   is the number of threads that have entered so far including the current
> +   thread).
> 
> s/so we the position/the position/ ?
> 
> +   adding COUNT to CURRENT_ROUND atomically.  Threads that belief that their
> +   round is not complete yet wait until CURRENT_ROUND is not smaller than
> +   their position anymore.
> 
> s/belief/believe/

Thanks for your review.  I fixed these.

> +	  if (i <= cr)
> +	    goto ready_to_leave;
> +	  else
> +	    break;
> 
> Is the else here only hit if the number of participating threads is
> greater than the barrier count?

Yes.  the surrounding block is only run if we finish previous rounds or
the current one, and if the CAS used for finishing it succeeds; if we
finished the current round (i <= cr), we're ready to leave; otherwise,
we finished a previous round, which in turn means that there must be
more threads trying to enter the barrier than the barrier count (which
isn't disallowed by POSIX).

Would you like to see a clarifying comment regarding this?

> Otherwise, it looks good to me, and seems like a good improvement to
> have. Though, a more experienced reviewer may have more to say. This
> is a bit more complicated than its predecessor. I'll test it on PPC
> next week.

Thanks!