Re: [RFC PATCH 2/2] RDMA/rxe: Add RDMA Atomic Write operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux