On 12/30/2021 7:14 AM, Xiao Yang wrote:
This patch implements RDMA Atomic Write operation for RC service. Signed-off-by: Xiao Yang <yangx.jy@xxxxxxxxxxx> --- drivers/infiniband/sw/rxe/rxe_comp.c | 4 +++ drivers/infiniband/sw/rxe/rxe_opcode.c | 18 ++++++++++ drivers/infiniband/sw/rxe/rxe_opcode.h | 3 ++ drivers/infiniband/sw/rxe/rxe_qp.c | 3 +- drivers/infiniband/sw/rxe/rxe_req.c | 10 ++++-- drivers/infiniband/sw/rxe/rxe_resp.c | 49 +++++++++++++++++++++----- include/rdma/ib_pack.h | 2 ++ include/rdma/ib_verbs.h | 2 ++ include/uapi/rdma/ib_user_verbs.h | 2 ++ 9 files changed, 81 insertions(+), 12 deletions(-)
<snip>
+/* Guarantee atomicity of atomic write operations at the machine level. */ +static DEFINE_SPINLOCK(atomic_write_ops_lock); + +static enum resp_states process_atomic_write(struct rxe_qp *qp, + struct rxe_pkt_info *pkt) +{ + u64 *src, *dst; + struct rxe_mr *mr = qp->resp.mr; + + src = payload_addr(pkt); + + dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, sizeof(u64)); + if (!dst || (uintptr_t)dst & 7) + return RESPST_ERR_MISALIGNED_ATOMIC; + + spin_lock_bh(&atomic_write_ops_lock); + *dst = *src; + spin_unlock_bh(&atomic_write_ops_lock);
Spinlock protection is insufficient! Using a spinlock protects only the atomicity of the store operation with respect to another remote atomic_write operation. But the semantics of RDMA_ATOMIC_WRITE go much further. The operation requires a fully atomic bus transaction, across the memory complex and with respect to CPU, PCI, and other sources. And this guarantee needs to apply to all architectures, including ones with noncoherent caches (software consistency). Because RXE is a software provider, I believe the most natural approach here is to use an atomic64_set(dst, *src). But I'm not certain this is supported on all the target architectures, therefore it may require some additional failure checks, such as stricter alignment than testing (dst & 7), or returning a failure if atomic64 operations are not available. I especially think the ARM and PPC platforms will need an expert review. IOW, nak! Tom.