util/pingpong: close mr after ep close

[shijin-aws]

pingpong doesn't support FI_MR_ENDPOINT today,
so the mr is associated with domain instead of ep. It is unsafe to close mr before closing ep because it can cause an EBUSY error when there are outstanding recvs of the mr posted to the ep/qp. This patch fixes this issue by moving the mr close after the ep close.

[shijin-aws]

I can implement a more comprehensive change like #8119 for this, but to my knowledge we likely won't support FI_MR_ENDPOINT for fi_pingpong, so I just make this simple change.

[aingerson]

I don't necessarily have an objection to this change but I'm trying to understand what outstanding operations this addresses - the finalize calls a send with FI_TRANSMIT_COMPLETE to consume the last rx and waits for the matching receive from the other side. Shouldn't all sends and receives be done with the MRs at the point of closing them?

[shijin-aws]

@aingerson there is 1 extra fi_recv posted in this test: https://github.com/ofiwg/libfabric/blob/main/util/pingpong.c#L2216-L2222 for dgram ep type, so there is always 1 extra fi_recv than the sends it will actually get. There is already 1 fi_recv after enable https://github.com/ofiwg/libfabric/blob/main/util/pingpong.c#L1558-L1559 which matches the send in the finalize call.

I don't fully understand why dgram pingpong has to be implemented in this way, especially fabtests fi_dgram_pingpong has such extra recv as well. I tried to remove such extra fi_recv, but then I will frequently get a cq read timeout.

[shijin-aws]

Is Intel CI failure related?

[aingerson]

@shijin-aws No, they were weird cluster issues. We don't test fi_pingpong in our CI. You can merge whenever you want, thanks!

[shijin-aws]

@shefty @j-xiong do you have any background for that 1 extra recv for dgram pingpong?

[shefty]

dgram_pingpong has this comment:

	/* Post an extra receive to avoid lacking a posted receive in the
	 * finalize.
	 */

The early receive likely handles a race where the sender sends a datagram prior to the receive being posted. You likely don't need an 'extra' receive, but one which is posted early. The very last receive which is posted could probably be removed, except it's hard to know which 'fi_recv' will actually be the last one.

[shijin-aws]

@shefty Thanks! I will keep such extra recv as is for now, and adjust the sequence of mr close in pingpong to be consistent with dgram_pingpong for non-FI_MR_ENDPOINT case.