RDMA/core: add rdma_get_dma_mr()

RFC PATCH In review Steve Wise 4 participants 13 messages Jun 25, 2015 – Jun 29, 2015 7 messages not in the archive InfiniBand

lore

Share & tools

lore

Steve Wise

author

2015-06-25 21:29 UTC #0 lore raw

RFC PATCH 1/1 RDMA/core: add rdma_get_dma_mr()

The semantics for MR access rights are not consistent across RDMA protocols. So rather than have applications try and glean what they need, have them pass in the intended roles for the MR to be allocated and let the RDMA core select the appropriate access rights given the roles and device capabilities.

This patch is just for reviewing the proposed API for allocating a dma mr in a transport-independent way. It is uncompiled/untested. If this looks ok for folks, then I'll incorporate it into the iSER/iWARP series and make use of it in isert.

Steve.

---
 drivers/infiniband/core/verbs.c |   34 ++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h         |   38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletions(-)

drivers/infiniband/core/verbs.c +34 -0

index bac3fb4..8a365d5 100644
@@ -1144,6 +1144,40 @@ struct ib_mr *ib_get_dma_mr(struct ib_pd *pd, int mr_access_flags)
 }
 EXPORT_SYMBOL(ib_get_dma_mr);
 
+struct ib_mr *rdma_get_dma_mr(struct ib_pd *pd, int mr_roles)
+{
+	int access_flags = 0;
+	struct ib_mr *mr;
+	int err;
+
+	if (mr_roles & RDMA_MRR_RECV)
+		access_flags |= IB_ACCESS_LOCAL_WRITE;
+
+	if (mr_roles & RDMA_MRR_WRITE_SINK)
+		access_flags |= IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE;
+
+	if (mr_roles & RDMA_MRR_READ_SINK) {
+		access_flags |= IB_ACCESS_LOCAL_WRITE;
+		if (rdma_protocol_iwarp(pd->device, rdma_start_port(pd->device))
+			access_flags |= IB_ACCESS_REMOTE_WRITE;
+	}
+
+	if (mr_roles & RDMA_MRR_ATOMIC)
+		access_flags |= IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_ATOMIC;
+
+	if (mr_roles & RDMA_MRR_MW_BIND)
+		access_flags |= IB_ACCESS_MW_BIND:
+
+	if (mr_roles & RDMA_MRR_ZERO_BASED)
+		access_flags |= IB_ACCESS_ZERO_BASED:
+
+	if (mr_roles & RDMA_MRR_ON_DEMAND)
+		access_flags |= IB_ACCESS_ON_DEMAND:
+
+	return ib_get_dma_mr(pd, access_flags);
+}
+EXPORT_SYMBOL(rdma_get_dma_mr);
+
 struct ib_mr *ib_reg_phys_mr(struct ib_pd *pd,
 			     struct ib_phys_buf *phys_buf_array,
 			     int num_phys_buf,

include/rdma/ib_verbs.h +37 -1

index 986fddb..feee869 100644
@@ -2494,7 +2494,43 @@ static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt)
 struct ib_mr *ib_get_dma_mr(struct ib_pd *pd, int mr_access_flags);
 
 /**
- * ib_dma_mapping_error - check a DMA addr for error
+ * rdma_mr_roles - possible roles a MR will be used for
+ *
+ * This allows a transport independent RDMA application to
+ * create MRs that are usable for all the desired roles w/o
+ * having to understand which access rights are needed.
+ */
+enum rdma_mr_roles {
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
+};
+
+/**
+ * rdma_get_dma_mr - Returns a memory region for system memory that is
+ * usable for DMA.
+ * @pd: The protection domain associated with the memory region.
+ * @mr_roles: Specifies the intended roles of the MR
+ *
+ * Use the intended roles from @mr_roles along with the device
+ * capabilities to define the needed access rights, and call
+ * ib_get_dma_mr() to allocate the MR.
+ *
+ * Note that the ib_dma_*() functions defined below must be used
+ * to create/destroy addresses used with the Lkey or Rkey returned
+ * by ib_get_dma_mr().
+ */
+struct ib_mr *rdma_get_dma_mr(struct ib_pd *pd, int mr_roles);
+
+/**
+ * rdma_dma_mapping_error - check a DMA addr for error
  * @dev: The device for which the dma_addr was created
  * @dma_addr: The DMA address to check
  */

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20150625212917.14869.66238.stgit-T4OLL4TyM9aNDNWfRnPdfg@public.gmane.org

Jason Gunthorpe

2015-06-25 21:39 UTC #2 lore raw

Quoted 8 lines · Steve Wise

- * ib_dma_mapping_error - check a DMA addr for error
+ * rdma_mr_roles - possible roles a MR will be used for
+ *
+ * This allows a transport independent RDMA application to
+ * create MRs that are usable for all the desired roles w/o
+ * having to understand which access rights are needed.
+ */
include/rdma/ib_verbs.h +1 -0
+enum rdma_mr_roles {

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2513

Quoted 10 lines

include/rdma/ib_verbs.h +10 -0

+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),

So we don't have this problem again, it would be best to explicitly list exactly which ib_wc_opcode members what use a RKEY are matched to which MMR flags

RDMA_MRR_RECV
LOCAL: post to receive work queue
RDMA_MRR_READ_SOURCE
LOCAL: post IB_WC_RDMA_READ to send work queue
RDMA_MRR_READ_SINK
REMOTE: respond to IB_WC_RDMA_READ
[..]

Seems reasonable to me otherwise

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20150625213944.GA27780-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

Steve Wise

author

2015-06-25 21:48 UTC #4 lore raw

Quoted 42 lines

-----Original Message-----
From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Thursday, June 25, 2015 4:40 PM
To: Steve Wise
Cc: sagig-VPRAkNaXOzVWk0Htik3J/[email protected]; roid-VPRAkNaXOzVWk0Htik3J/[email protected]; ogerlitz-VPRAkNaXOzVWk0Htik3J/[email protected]; [email protected]; [email protected]
Subject: Re: [PATCH RFC] RDMA/core: add rdma_get_dma_mr()
Quoted 8 lines · Steve Wise
- * ib_dma_mapping_error - check a DMA addr for error
+ * rdma_mr_roles - possible roles a MR will be used for
+ *
+ * This allows a transport independent RDMA application to
+ * create MRs that are usable for all the desired roles w/o
+ * having to understand which access rights are needed.
+ */
include/rdma/ib_verbs.h +1 -0
+enum rdma_mr_roles {
include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2513
Quoted 10 lines
include/rdma/ib_verbs.h +10 -0
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
So we don't have this problem again, it would be best to explicitly list exactly which ib_wc_opcode members what use a RKEY are matched to which MMR flags
RDMA_MRR_RECV
LOCAL: post to receive work queue
RDMA_MRR_READ_SOURCE
LOCAL: post IB_WC_RDMA_READ to send work queue
RDMA_MRR_READ_SINK
REMOTE: respond to IB_WC_RDMA_READ
[..]

Quoted 2 lines

Seems reasonable to me otherwise

Good idea.

Thanks for the reviews!

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hefty, Sean

2015-06-25 22:37 UTC #5 lore raw

In reply to a message not in the archive

PiArZW51bSByZG1hX21yX3JvbGVzIHsNCg0KSSB3b3VsZCBkcm9wIG5hbWluZyB0aGUgZW51bSAt IGl0IHNob3VsZG4ndCBiZSB1c2VkLCBhcyB0aGUgdmFsdWVzIGFyZSBiaXQgZmxhZ3MuDQoNCj4g KwlSRE1BX01SUl9SRUNWCQkJPSAxLA0KPiArCVJETUFfTVJSX1NFTkQJCQk9ICgxPDwxKSwNCj4g KwlSRE1BX01SUl9SRUFEX1NPVVJDRQkJPSAoMTw8MiksDQo+ICsJUkRNQV9NUlJfUkVBRF9TSU5L CQk9ICgxPDwzKSwNCj4gKwlSRE1BX01SUl9XUklURV9TT1VSQ0UJCT0gKDE8PDQpLA0KPiArCVJE TUFfTVJSX1dSSVRFX1NJTksJCT0gKDE8PDUpLA0KPiArCVJETUFfTVJSX0FUT01JQwkJCT0gKDE8 PDYpLA0KPiArCVJETUFfTVJSX01XX0JJTkQJCT0gKDE8PDcpLA0KPiArCVJETUFfTVJSX1pFUk9f QkFTRUQJCT0gKDE8PDgpLA0KDQpUaGVyZSdzICdzb21ldGhpbmcnIGRpZmZlcmVudCBhYm91dCB0 aGlzIHJvbGUgdGhhdCBjYXVzZSBtZSBoZXNpdGF0aW9uLiAgTWF5YmUgdGhhdCBpdCdzIGRlcGVu ZGVudCBvbiBvdGhlciByb2xlcyBiZWluZyBzZXQgdG8gYmUgdXNlZnVsPyAgSSdtIG5vdCBzdXJl Lg0KDQpNYXliZSB3ZSBuZWVkIGJvdGggcm9sZXMgYW5kIHJlZ2lzdHJhdGlvbiBmbGFncywgd2l0 aCB0aGlzIGRlY2xhcmVkIGFzIGEgZmxhZz8NCg0KPiArCVJETUFfTVJSX0FDQ0VTU19PTl9ERU1B TkQJPSAoMTw8OSksDQoNClRoaXMgb25lIGlzIGV2ZW4gbW9yZSBkaWZmZXJlbnQsIGFzIGl0IGRv ZXNuJ3QgaW1wYWN0IGhvdyB0aGUgTVIgaW50ZXJhY3RzIHdpdGggdGhlIGludGVyZmFjZXMsIG9y IGNoYW5nZSBob3cgdGhlIGFwcGxpY2F0aW9uIHVzZXMgdGhlIE1SLiAgVGhpcyBpcyByZWFsbHkg YSBoaW50IHRvIHRoZSBwcm92aWRlciByZWdhcmRpbmcgdGhlIHNlbGVjdGlvbiBvZiBkaWZmZXJl bnQgaW1wbGVtZW50YXRpb24gZmxvd3MuDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1828884A29C6694DAF28B7E6B8A82373A8FF9ECA-P5GAC/[email protected]

Steve Wise

author

2015-06-26 14:02 UTC #7 lore raw

Quoted 2 lines · Sean

Quoted 1 line
+enum rdma_mr_roles {
I would drop naming the enum - it shouldn't be used, as the values are bit flags.

ok.

Quoted 11 lines

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2512
Quoted 9 lines
include/rdma/ib_verbs.h +9 -0
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
There's 'something' different about this role that cause me hesitation. Maybe that it's dependent on other roles being set to be useful? I'm not sure.

Quoted 4 lines

Maybe we need both roles and registration flags, with this declared as a flag?
include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2513
Quoted 1 line
include/rdma/ib_verbs.h +1 -0
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
This one is even more different, as it doesn't impact how the MR interacts with the interfaces, or change how the application uses the MR. This is really a hint to the provider regarding the selection of different implementation flows.

How about roles and attributes? ZERO_BASED and ACCESS_ON_DEMAND would be attributes and the rest roles.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 558D5B6E.6090900-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org

Steve Wise

author

2015-06-26 15:18 UTC #9 lore raw

Quoted 15 lines · Steve Wise

Quoted 2 lines · Sean

Quoted 1 line
+enum rdma_mr_roles {
I would drop naming the enum - it shouldn't be used, as the values are bit flags.

ok.

Quoted 9 lines

include/rdma/ib_verbs.h +9 -0

+    RDMA_MRR_RECV            = 1,
+    RDMA_MRR_SEND            = (1<<1),
+    RDMA_MRR_READ_SOURCE        = (1<<2),
+    RDMA_MRR_READ_SINK        = (1<<3),
+    RDMA_MRR_WRITE_SOURCE        = (1<<4),
+    RDMA_MRR_WRITE_SINK        = (1<<5),
+    RDMA_MRR_ATOMIC            = (1<<6),
+    RDMA_MRR_MW_BIND        = (1<<7),
+    RDMA_MRR_ZERO_BASED        = (1<<8),

Quoted 9 lines

Quoted 6 lines
There's 'something' different about this role that cause me hesitation. Maybe that it's dependent on other roles being set to be useful? I'm not sure.
Maybe we need both roles and registration flags, with this declared as a flag?
Quoted 1 line
+ RDMA_MRR_ACCESS_ON_DEMAND = (1<<9),
This one is even more different, as it doesn't impact how the MR interacts with the interfaces, or change how the application uses the MR. This is really a hint to the provider regarding the selection of different implementation flows.
How about roles and attributes? ZERO_BASED and ACCESS_ON_DEMAND would be attributes and the rest roles.

I'm thinking now about the access flags specified in a IB_WR_FAST_REG_MR work request. I think we need a similar function for this, but it will return the ib_access_flags bits to be stored in ib_send_wr->wr.fast_reg.access_flags field. With this function defined, then rdma_get_dma_mr() can use it as well.

So perhaps an internal static function in verbs.c that does the bulk of what I put in rdma_get_dma_mr() originally called:

device_mr_access_flags(device, roles, attrs)

used by exported functions (or inlines in ib_verbs.h):

rdma_get_dma_mr(device, roles, attrs)
rdma_fast_reg_access_flags(device, roles, attrs)

I'll code this up and send for another round of review. Eventually i'll get this included in the series with the iSER/iWARP patches for testing/further review.

Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Haggai Eran

2015-06-28 15:44 UTC #10 lore raw

In reply to a message not in the archive

Quoted 2 lines · Sean

Quoted 1 line
include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2513
Quoted 1 line
include/rdma/ib_verbs.h +1 -0
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
This one is even more different, as it doesn't impact how the MR interacts with the interfaces, or change how the application uses the MR. This is really a hint to the provider regarding the selection of different implementation flows.

I don't think this bit is relevant at all for DMA MR access flags. It is only used to allow page faults on an MR, so it is only relevant when creating a user-space MR.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Haggai Eran

2015-06-28 15:47 UTC #11 lore raw

In reply to a message not in the archive

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2507

Quoted 5 lines · Steve Wise

include/rdma/ib_verbs.h +5 -0

+enum rdma_mr_roles {
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),

Maybe it's just me, but it took me a second to figure out which was the source and which was the sink in RDMA reads. Do you think calling them initiator and responder/target would be better?

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2514

Quoted 7 lines

include/rdma/ib_verbs.h +7 -0

+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
+};

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 55901703.3050001-VPRAkNaXOzVWk0Htik3J/[email protected]

Steve Wise

author

2015-06-29 13:47 UTC #13 lore raw

Quoted 10 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Haggai Eran
Sent: Sunday, June 28, 2015 10:47 AM
To: Steve Wise; jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Cc: sagig-VPRAkNaXOzVWk0Htik3J/[email protected]; roid-VPRAkNaXOzVWk0Htik3J/[email protected]; ogerlitz-VPRAkNaXOzVWk0Htik3J/[email protected]; [email protected]; [email protected]
Subject: Re: [PATCH RFC] RDMA/core: add rdma_get_dma_mr()
On 26/06/2015 00:29, Steve Wise wrote:

Quoted 9 lines

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2507
Quoted 5 lines
include/rdma/ib_verbs.h +5 -0
+enum rdma_mr_roles {
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
Maybe it's just me, but it took me a second to figure out which was the source and which was the sink in RDMA reads. Do you think calling them initiator and responder/target would be better?

Not to me. For an RDMA operation, the "initiator" is the app that issues the read request WR. That app doesn't create what I call the READ_SOURCE MR. Its peer application does. So calling READ_SOURCE something like READ_INITIATOR doesn't make sense to me. That's why I thought SOURCE and SINK were clearer. Perhaps not...

I have a new version I'll send out soon that will comment all of these in the enum declaration. Perhaps that will make it clear.

Quoted 13 lines

include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2514
Quoted 8 lines
include/rdma/ib_verbs.h +7 -0
+	RDMA_MRR_WRITE_SOURCE		= (1<<4),
+	RDMA_MRR_WRITE_SINK		= (1<<5),
+	RDMA_MRR_ATOMIC			= (1<<6),
+	RDMA_MRR_MW_BIND		= (1<<7),
+	RDMA_MRR_ZERO_BASED		= (1<<8),
+	RDMA_MRR_ACCESS_ON_DEMAND	= (1<<9),
+};
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Jason Gunthorpe

2015-06-29 15:55 UTC #14 lore raw

Quoted 11 lines · Steve Wise

Quoted 10 lines · Steve Wise
include/rdma/ib_verbs.h static inline int ib_req_ncomp_notif(struct ib_cq *cq, int wc_cnt) · line 2507
Quoted 5 lines
include/rdma/ib_verbs.h +5 -0
+enum rdma_mr_roles {
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
Maybe it's just me, but it took me a second to figure out which was the source and which was the sink in RDMA reads. Do you think calling them initiator and responder/target would be better?

Quoted 5 lines

Not to me. For an RDMA operation, the "initiator" is the app that issues the read request WR. That app doesn't create what I call the READ_SOURCE MR. Its peer application does. So calling READ_SOURCE something like READ_INITIATOR doesn't make sense to me. That's why I thought SOURCE and SINK were clearer. Perhaps not...

I would call SOURCE the RESPONDER..

Initiator/Poster and Responder is closest to the language used in other places in the API for READ.

Quoted 2 lines

I have a new version I'll send out soon that will comment all of these in the enum declaration. Perhaps that will make it clear.

I think once you have a comment with the table mapping to allowed local/remote verbs then things will be fine with whatever names you pick.

Honestly, the names that IBA picked were fine and descriptive too, the only problem is that the iwarp hardware can't implement the IBA names :(

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20150629155503.GA2755-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

Hefty, Sean

2015-06-29 16:08 UTC #16 lore raw

Quoted 17 lines

Quoted 7 lines
Quoted 7 lines
Review
Quoted 5 lines
(quoted diff) +5 -0
+enum rdma_mr_roles {
+	RDMA_MRR_RECV			= 1,
+	RDMA_MRR_SEND			= (1<<1),
+	RDMA_MRR_READ_SOURCE		= (1<<2),
+	RDMA_MRR_READ_SINK		= (1<<3),
Maybe it's just me, but it took me a second to figure out which was
the
Quoted 8 lines
Quoted 2 lines
source and which was the sink in RDMA reads. Do you think calling them initiator and responder/target would be better?
Not to me. For an RDMA operation, the "initiator" is the app that issues the read request WR. That app doesn't create what I call the READ_SOURCE MR. Its peer application does. So calling READ_SOURCE something like READ_INITIATOR doesn't make sense to me. That's why I thought SOURCE and SINK were clearer. Perhaps not...

Quoted 4 lines

I would call SOURCE the RESPONDER..
Initiator/Poster and Responder is closest to the language used in other places in the API for READ.

I agree with Steve. I think of this in terms of copy and like source/dest myself, similar to source/sink. A memory buffer is not a responder or initiator.

In hindsight, we probably never should have exposed initiator_depth and responder_resources. Those values have been an endless source of confusion and bugs. --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Haggai Eran

2015-06-28 16:01 UTC #17 lore raw

In reply to a message not in the archive

Quoted 5 lines · Steve Wise

The semantics for MR access rights are not consistent across RDMA protocols. So rather than have applications try and glean what they need, have them pass in the intended roles for the MR to be allocated and let the RDMA core select the appropriate access rights given the roles and device capabilities.

I wanted to point out that with this scheme the ULP may sometimes get an MR with a wider set of permissions that it asked for, and I'm not sure that's always safe. Perhaps the ULP wants to guarantee that the MR doesn't allow certain kinds of accesses and doesn't expect the verbs layer to change that.

This patch looks fine since it is just a helper function, but if you replace the MR creation with these flags, you might confuse users to think that they can control e.g. write sink and read sink separately.

Haggai

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 55901A44.8020103-VPRAkNaXOzVWk0Htik3J/[email protected]

Hefty, Sean

2015-06-29 01:31 UTC #19 lore raw

PiBJIHdhbnRlZCB0byBwb2ludCBvdXQgdGhhdCB3aXRoIHRoaXMgc2NoZW1lIHRoZSBVTFAgbWF5 IHNvbWV0aW1lcyBnZXQgYW4NCj4gTVIgd2l0aCBhIHdpZGVyIHNldCBvZiBwZXJtaXNzaW9ucyB0 aGF0IGl0IGFza2VkIGZvciwgYW5kIEknbSBub3Qgc3VyZQ0KPiB0aGF0J3MgYWx3YXlzIHNhZmUu IFBlcmhhcHMgdGhlIFVMUCB3YW50cyB0byBndWFyYW50ZWUgdGhhdCB0aGUgTVINCj4gZG9lc24n dCBhbGxvdyBjZXJ0YWluIGtpbmRzIG9mIGFjY2Vzc2VzIGFuZCBkb2Vzbid0IGV4cGVjdCB0aGUg dmVyYnMNCj4gbGF5ZXIgdG8gY2hhbmdlIHRoYXQuDQoNClRoZSBwcm92aWRlcnMgY2FuIHJldHVy biB0aGUgYWN0dWFsIHBlcm1pc3Npb25zIHRoYXQgd2VyZSBncmFudGVkLiAgSWYgdGhlIGFwcCBj YXJlcywgaXQgY2FuIGNoZWNrLg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html