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

[Ralph Campbell]

On Wed, 2008-02-06 at 14:37 -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.

Probably the safer thing to do is use the qp->s_lock since the
receive interrupt handler is the producer and the send tasklet
which sends the RDMA read or ATOMIC response is the consumer.
I will create a patch and send it out...