[ofa-general] Re: Dubious use of barrier() in ipath

[Arthur Jones]

hi roland, thanks, i've forwarded this to
ralph (the author), i couldn't follow at
first glance, but i bet he'll remember...

arthur

On Wed, Feb 06, 2008 at 02:37:06PM -0800, Roland Dreier wrote:
> In ipath_rc.c, there are a couple of places that do:
> 
> 		qp->r_msn++;
> 		qp->r_psn++;
> 		qp->r_state = opcode;
> 		qp->r_nak_state = 0;
> 		barrier();
> 		qp->r_head_ack_queue = next;
> 
> This looks pretty suspicious to me -- I haven't really tried to
> understand the code, but it has the flavor of protecting against
> another CPU seeing the r_head_ack_queue update before the other
> updates; and barrier() doesn't actually do that.  If this code is
> correct, I think a comment explaining the barrier() would be good.
> 
> But I have the feeling that the barrier() should really be wmb(), with
> rmb()s added on the reader side.
> 
>  - R.