| Message ID | 20180706171736.14543-2-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Jul 06, 2018 at 10:17:24AM -0700, Bart Van Assche wrote: > Since the C programming language does not allow to define a single > function of which the constness of the return type depends on the > constness of an argument type, convert the struct ib_send_wr > conversion functions into macros. That will allow these conversion > functions to be used both in a context where the send work request > is modified and where it is not modified. The iSER initiator driver > is the only driver that uses one of these functions to obtain a > modifiable pointer to a work request, namely as follows: This seems confounding. container_of is not const preserving, so it happily strips the const off the input pointer, which is what the inline wrappers would do if they were marked to accept a const - so why convert them to #define's? I'm actually a bit surprised this works, I think it might be a bug in the container_of macro's type checking that it is ignoring the const qualifier on the input pointer - not totally sure I want to rely on that I'd like this alot better if container_of could propogate const, but can't think of a way to achieve that. Considering these limitations, why are you making this series? Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gRnJpLCAyMDE4LTA3LTA2IGF0IDExOjMzIC0wNjAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IGNvbnRhaW5lcl9vZiBpcyBub3QgY29uc3QgcHJlc2VydmluZywgc28gaXQgaGFwcGlseSBz dHJpcHMgdGhlIGNvbnN0DQo+IG9mZiB0aGUgaW5wdXQgcG9pbnRlciwgd2hpY2ggaXMgd2hhdCB0 aGUgaW5saW5lIHdyYXBwZXJzIHdvdWxkIGRvIGlmIHRoZXkNCj4gd2VyZSBtYXJrZWQgdG8gYWNj ZXB0IGEgY29uc3QgLSBzbyB3aHkgY29udmVydCB0aGVtIHRvICNkZWZpbmUncz8NCg0KQXMgbWVu dGlvbmVkIGluIHRoZSBkZXNjcmlwdGlvbiBvZiBwYXRjaCAxLzEzLCB0aGVyZSBpcyBjb2RlIHVw c3RyZWFtIHRoYXQNCm1vZGlmaWVzIHRoZSBkYXRhIHRoZSByZXR1cm5lZCBwb2ludGVyIHBvaW50 cyBhdC4NCg0KPiBJJ20gYWN0dWFsbHkgYSBiaXQgc3VycHJpc2VkIHRoaXMgd29ya3MsIEkgdGhp bmsgaXQgbWlnaHQgYmUgYSBidWcgaW4NCj4gdGhlIGNvbnRhaW5lcl9vZiBtYWNybydzIHR5cGUg Y2hlY2tpbmcgdGhhdCBpdCBpcyBpZ25vcmluZyB0aGUgY29uc3QNCj4gcXVhbGlmaWVyIG9uIHRo ZSBpbnB1dCBwb2ludGVyIC0gbm90IHRvdGFsbHkgc3VyZSBJIHdhbnQgdG8gcmVseSBvbiB0aGF0 DQo+DQo+IEknZCBsaWtlIHRoaXMgYWxvdCBiZXR0ZXIgaWYgY29udGFpbmVyX29mIGNvdWxkIHBy b3BvZ2F0ZSBjb25zdCwgYnV0DQo+IGNhbid0IHRoaW5rIG9mIGEgd2F5IHRvIGFjaGlldmUgdGhh dC4NCg0KSG93IGFib3V0IGtlZXBpbmcgdGhlIGlubGluZSBmdW5jdGlvbnMsIHRvIGRlY2xhcmUg dGhlIGFyZ3VtZW50IGFuZCByZXR1cm4NCnR5cGVzIGNvbnN0LCBhbmQgdG8gdXNlIGNvbnRhaW5l cl9vZigpIGRpcmVjdGx5IGluIHRoZSBpU0VSIGRyaXZlcj8NCg0KPiBDb25zaWRlcmluZyB0aGVz ZSBsaW1pdGF0aW9ucywgd2h5IGFyZSB5b3UgbWFraW5nIHRoaXMgc2VyaWVzPw0KDQpUbyBpbXBy b3ZlIHJlYWRhYmlsaXR5IG9mIFJETUEgZHJpdmVyIGNvZGUuIFRoaXMgcGF0Y2ggc2VyaWVzIGlz IHNvbWV0aGluZw0KSSBjYW1lIHVwIHdpdGggd2hpbGUgYW5hbHl6aW5nIGEgS0FTQU4gY29tcGxh aW50IGFib3V0IHRoZSByZG1hX3J4ZSBmdW5jdGlvbg0KaW5pdF9zZW5kX3dyKCkuIFRoZSBjb25z dCBhbm5vdGF0aW9ucyBtYWtlIGl0IG1vcmUgY2xlYXIgd2hpY2ggd29yayByZXF1ZXN0DQpwb2lu dGVyIGlzIHRoZSBpbnB1dCB3b3JrIHJlcXVlc3QgYW5kIHdoaWNoIG9uZSBpcyB0aGUgb3V0cHV0 IHdvcmsgcmVxdWVzdC4NCg0KVGhhbmtzLA0KDQpCYXJ0Lg0KDQoNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 06, 2018 at 06:08:59PM +0000, Bart Van Assche wrote: > On Fri, 2018-07-06 at 11:33 -0600, Jason Gunthorpe wrote: > > container_of is not const preserving, so it happily strips the const > > off the input pointer, which is what the inline wrappers would do if they > > were marked to accept a const - so why convert them to #define's? > > As mentioned in the description of patch 1/13, there is code upstream that > modifies the data the returned pointer points at. > > > I'm actually a bit surprised this works, I think it might be a bug in > > the container_of macro's type checking that it is ignoring the const > > qualifier on the input pointer - not totally sure I want to rely on that > > > > I'd like this alot better if container_of could propogate const, but > > can't think of a way to achieve that. > > How about keeping the inline functions, to declare the argument and return > types const, and to use container_of() directly in the iSER driver? I think I would prefer that, at least it is const preserving, not const discarding that way. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index e1130c6c1377..f0b70f3f2a65 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1373,10 +1373,7 @@ struct ib_rdma_wr { u32 rkey; }; -static inline struct ib_rdma_wr *rdma_wr(struct ib_send_wr *wr) -{ - return container_of(wr, struct ib_rdma_wr, wr); -} +#define rdma_wr(send_wr) container_of(send_wr, struct ib_rdma_wr, wr) struct ib_atomic_wr { struct ib_send_wr wr; @@ -1388,10 +1385,7 @@ struct ib_atomic_wr { u32 rkey; }; -static inline struct ib_atomic_wr *atomic_wr(struct ib_send_wr *wr) -{ - return container_of(wr, struct ib_atomic_wr, wr); -} +#define atomic_wr(send_wr) container_of(send_wr, struct ib_atomic_wr, wr) struct ib_ud_wr { struct ib_send_wr wr; @@ -1405,10 +1399,7 @@ struct ib_ud_wr { u8 port_num; /* valid for DR SMPs on switch only */ }; -static inline struct ib_ud_wr *ud_wr(struct ib_send_wr *wr) -{ - return container_of(wr, struct ib_ud_wr, wr); -} +#define ud_wr(send_wr) container_of(send_wr, struct ib_ud_wr, wr) struct ib_reg_wr { struct ib_send_wr wr; @@ -1417,10 +1408,7 @@ struct ib_reg_wr { int access; }; -static inline struct ib_reg_wr *reg_wr(struct ib_send_wr *wr) -{ - return container_of(wr, struct ib_reg_wr, wr); -} +#define reg_wr(send_wr) container_of(send_wr, struct ib_reg_wr, wr) struct ib_sig_handover_wr { struct ib_send_wr wr; @@ -1430,10 +1418,8 @@ struct ib_sig_handover_wr { struct ib_sge *prot; }; -static inline struct ib_sig_handover_wr *sig_handover_wr(struct ib_send_wr *wr) -{ - return container_of(wr, struct ib_sig_handover_wr, wr); -} +#define sig_handover_wr(send_wr) \ + container_of(send_wr, struct ib_sig_handover_wr, wr) struct ib_recv_wr { struct ib_recv_wr *next;
Since the C programming language does not allow to define a single function of which the constness of the return type depends on the constness of an argument type, convert the struct ib_send_wr conversion functions into macros. That will allow these conversion functions to be used both in a context where the send work request is modified and where it is not modified. The iSER initiator driver is the only driver that uses one of these functions to obtain a modifiable pointer to a work request, namely as follows: wr = reg_wr(iser_tx_next_wr(tx_desc)); Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Cc: Leon Romanovsky <leonro@mellanox.com> Cc: Parav Pandit <parav@mellanox.com> --- include/rdma/ib_verbs.h | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-)