[PATCH V2 2/2] mlx4/IB: Add support for enhanced atomic operations

[PATCH V2 2/2] mlx4/IB: Add support for enhanced atomic operations

[Vladimir Sokolovsky]

Added support for masked atomic operations:
Masked Compare and Swap (MskCmpSwap)
The MskCmpSwap atomic operation is an extension to the CmpSwap operation
defined in the IB spec. MskCmpSwap allows the user to select a portion of the
64 bit target data for the “compare” check as well as to restrict the swap to a
(possibly different) portion. The pseudo code below describes the operation:

| atomic_response = *va
| if (((cmp XOR *va) AND cmp_mask) is ZERO) then
|     *va = (*va AND NOT(swap_mask)) OR (swap AND swap_mask)
|
| return atomic_response

The additional operands are carried in the Extended Transport Header. Atomic
response generation and packet format for MskCmpSwap is as for standard IB
Atomic operations.

Masked Fetch and Add (MFetchAdd)
The MFetchAdd Atomic operation extends the functionality of the standard IB
FetchAdd by allowing the user to split the target into multiple fields of
selectable length. The atomic add is done independently on each one of this
fields. A bit set in the field_boundary parameter specifies the field
boundaries. The pseudo code below describes the operation:

| bit_adder(ci, b1, b2, *co)
| {
|	value = ci + b1 + b2
|	*co = !!(value & 2)
|
|	return value & 1
| }
|
| #define MASK_IS_SET(mask, attr)      (!!((mask)&(attr)))
| bit_position = 1
| carry = 0
| atomic_response = 0
|
| for i = 0 to 63
| {
|         if ( i != 0 )
|                 bit_position =  bit_position << 1
|
|         bit_add_res = bit_adder(carry, MASK_IS_SET(*va, bit_position), MASK_IS_SET(add_value, bit_position), &new_carry)
|         if (bit_add_res)
|                 atomic_response |= bit_position
|
|         carry = ((new_carry) && (!MASK_IS_SET(fa_mask, bit_position)))
| }
|
| return atomic_response

Signed-off-by: Vladimir Sokolovsky <vlad-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/cq.c   |    8 ++++++++
 drivers/infiniband/hw/mlx4/main.c |    2 ++
 drivers/infiniband/hw/mlx4/qp.c   |   27 +++++++++++++++++++++++++++
 include/linux/mlx4/device.h       |    4 ++--
 include/linux/mlx4/qp.h           |    7 +++++++
 5 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
index de5263b..8dd451e 100644
--- a/drivers/infiniband/hw/mlx4/cq.c
+++ b/drivers/infiniband/hw/mlx4/cq.c
@@ -660,6 +660,14 @@ repoll:
 			wc->opcode    = IB_WC_FETCH_ADD;
 			wc->byte_len  = 8;
 			break;
+		case MLX4_OPCODE_ATOMIC_MASKED_CS:
+			wc->opcode    = IB_WC_MASKED_COMP_SWAP;
+			wc->byte_len  = 8;
+			break;
+		case MLX4_OPCODE_ATOMIC_MASKED_FA:
+			wc->opcode    = IB_WC_MASKED_FETCH_ADD;
+			wc->byte_len  = 8;
+			break;
 		case MLX4_OPCODE_BIND_MW:
 			wc->opcode    = IB_WC_BIND_MW;
 			break;
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index e596537..60e1174 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -112,6 +112,8 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
 	    (dev->dev->caps.bmme_flags & MLX4_BMME_FLAG_FAST_REG_WR))
 		props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS;
 
+	props->device_cap_flags |= IB_DEVICE_MASKED_ATOMIC;
+
 	props->vendor_id	   = be32_to_cpup((__be32 *) (out_mad->data + 36)) &
 		0xffffff;
 	props->vendor_part_id	   = be16_to_cpup((__be16 *) (out_mad->data + 30));
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 2a97c96..51e6a29 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -84,6 +84,8 @@ static const __be32 mlx4_ib_opcode[] = {
 	[IB_WR_SEND_WITH_INV]		= cpu_to_be32(MLX4_OPCODE_SEND_INVAL),
 	[IB_WR_LOCAL_INV]		= cpu_to_be32(MLX4_OPCODE_LOCAL_INVAL),
 	[IB_WR_FAST_REG_MR]		= cpu_to_be32(MLX4_OPCODE_FMR),
+	[IB_WR_ATOMIC_MASKED_CMP_AND_SWP]	= cpu_to_be32(MLX4_OPCODE_ATOMIC_MASKED_CS),
+	[IB_WR_ATOMIC_MASKED_FETCH_AND_ADD]	= cpu_to_be32(MLX4_OPCODE_ATOMIC_MASKED_FA),
 };
 
 static struct mlx4_ib_sqp *to_msqp(struct mlx4_ib_qp *mqp)
@@ -1406,6 +1408,9 @@ static void set_atomic_seg(struct mlx4_wqe_atomic_seg *aseg, struct ib_send_wr *
 	if (wr->opcode == IB_WR_ATOMIC_CMP_AND_SWP) {
 		aseg->swap_add = cpu_to_be64(wr->wr.atomic.swap);
 		aseg->compare  = cpu_to_be64(wr->wr.atomic.compare_add);
+	} else if (wr->opcode == IB_WR_ATOMIC_MASKED_FETCH_AND_ADD) {
+		aseg->swap_add = cpu_to_be64(wr->wr.atomic.compare_add);
+		aseg->compare  = cpu_to_be64(wr->wr.atomic.compare_add_mask);
 	} else {
 		aseg->swap_add = cpu_to_be64(wr->wr.atomic.compare_add);
 		aseg->compare  = 0;
@@ -1413,6 +1418,14 @@ static void set_atomic_seg(struct mlx4_wqe_atomic_seg *aseg, struct ib_send_wr *
 
 }
 
+static void set_mask_atomic_seg(struct mlx4_wqe_mask_atomic_seg *aseg, struct ib_send_wr *wr)
+{
+	aseg->swap_add = cpu_to_be64(wr->wr.atomic.swap);
+	aseg->swap_add_mask = cpu_to_be64(wr->wr.atomic.swap_mask);
+	aseg->compare  = cpu_to_be64(wr->wr.atomic.compare_add);
+	aseg->compare_mask = cpu_to_be64(wr->wr.atomic.compare_add_mask);
+}
+
 static void set_datagram_seg(struct mlx4_wqe_datagram_seg *dseg,
 			     struct ib_send_wr *wr)
 {
@@ -1566,6 +1579,7 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 			switch (wr->opcode) {
 			case IB_WR_ATOMIC_CMP_AND_SWP:
 			case IB_WR_ATOMIC_FETCH_AND_ADD:
+			case IB_WR_ATOMIC_MASKED_FETCH_AND_ADD:
 				set_raddr_seg(wqe, wr->wr.atomic.remote_addr,
 					      wr->wr.atomic.rkey);
 				wqe  += sizeof (struct mlx4_wqe_raddr_seg);
@@ -1578,6 +1592,19 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 
 				break;
 
+			case IB_WR_ATOMIC_MASKED_CMP_AND_SWP:
+				set_raddr_seg(wqe, wr->wr.atomic.remote_addr,
+					      wr->wr.atomic.rkey);
+				wqe  += sizeof (struct mlx4_wqe_raddr_seg);
+
+				set_mask_atomic_seg(wqe, wr);
+				wqe  += sizeof (struct mlx4_wqe_mask_atomic_seg);
+
+				size += (sizeof (struct mlx4_wqe_raddr_seg) +
+					 sizeof (struct mlx4_wqe_mask_atomic_seg)) / 16;
+
+				break;
+
 			case IB_WR_RDMA_READ:
 			case IB_WR_RDMA_WRITE:
 			case IB_WR_RDMA_WRITE_WITH_IMM:
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index e92d1bf..efeb1dd 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -123,8 +123,8 @@ enum {
 	MLX4_OPCODE_RDMA_READ		= 0x10,
 	MLX4_OPCODE_ATOMIC_CS		= 0x11,
 	MLX4_OPCODE_ATOMIC_FA		= 0x12,
-	MLX4_OPCODE_ATOMIC_MASK_CS	= 0x14,
-	MLX4_OPCODE_ATOMIC_MASK_FA	= 0x15,
+	MLX4_OPCODE_ATOMIC_MASKED_CS	= 0x14,
+	MLX4_OPCODE_ATOMIC_MASKED_FA	= 0x15,
 	MLX4_OPCODE_BIND_MW		= 0x18,
 	MLX4_OPCODE_FMR			= 0x19,
 	MLX4_OPCODE_LOCAL_INVAL		= 0x1b,
diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
index 9f29d86..1a48413 100644
--- a/include/linux/mlx4/qp.h
+++ b/include/linux/mlx4/qp.h
@@ -285,6 +285,13 @@ struct mlx4_wqe_atomic_seg {
 	__be64			compare;
 };
 
+struct mlx4_wqe_mask_atomic_seg {
+	__be64			swap_add;
+	__be64			compare;
+	__be64			swap_add_mask;
+	__be64			compare_mask;
+};
+
 struct mlx4_wqe_data_seg {
 	__be32			byte_count;
 	__be32			lkey;
-- 
1.6.6.GIT

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] mlx4/IB: Add support for enhanced atomic operations

[Roland Dreier]

 > | atomic_response = *va
 > | if (((cmp XOR *va) AND cmp_mask) is ZERO) then
 > |     *va = (*va AND NOT(swap_mask)) OR (swap AND swap_mask)
 > |
 > | return atomic_response

This is a great, terse description of the masked compare and swap
operation.  Do you think it would be more readable with C operations (ie
^ for XOR, & for AND, ~ for NOT, and | for OR)?  Also, I think it would
be good to use the exact field names as the work request field you add
(ie compare_add and compare_add_mask instead of cmp and cmp_mask).

(And as I said before, this belongs with the generic patch, not the mlx4
patch description)

 > Masked Fetch and Add (MFetchAdd)
 > The MFetchAdd Atomic operation extends the functionality of the standard IB
 > FetchAdd by allowing the user to split the target into multiple fields of
 > selectable length. The atomic add is done independently on each one of this
 > fields. A bit set in the field_boundary parameter specifies the field
 > boundaries. The pseudo code below describes the operation:

This is a bit harder to read, but I guess the operation is harder to
describe.  But what is really missing is the statement that the relevant
fields in the wr are compare_add and compare_add_mask.  Failing that,
it's really hard to know how to use this operation without looking at
the mlx4 source code.

 > +	props->device_cap_flags |= IB_DEVICE_MASKED_ATOMIC;

Is there no firmware dependency here?  If this really should be set
unconditionally, then please just roll it into the other flags that get
set for all devices.
-- 
Roland Dreier  <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] mlx4/IB: Add support for enhanced atomic operations

[Tziporet Koren]

On 2/26/2010 1:44 AM, Roland Dreier wrote:
> Is there no firmware dependency here?  If this really should be set
> unconditionally, then please just roll it into the other flags that get
> set for all devices.
>    
There is no FW dependency here and it's working from first FW release of 
ConnectX.
We have added it to the driver just now since there was no request 
before and now RDS wish to use it.

Tziporet
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html