On 3/11/2014 4:50 AM, Jason Gunthorpe wrote:
On Sun, Nov 02, 2014 at 11:30:05AM +0200, Or Gerlitz wrote:
@@ -886,7 +887,7 @@ struct ibv_context_ops {
int (*dealloc_pd)(struct ibv_pd *pd);
struct ibv_mr * (*reg_mr)(struct ibv_pd *pd, void *addr, size_t
length,
int access);
- struct ibv_mr * (*rereg_mr)(struct ibv_mr *mr,
+ int (*rereg_mr)(struct ibv_mr *mr,
int flags,
struct ibv_pd *pd, void *addr,
size_t length,
What is going on here?
Signatures in the ops structure should not be changed without quite a
bit more explanation. I'd like to see this in a dedicated patch with a
proper discussion about why it is safe...
(I admit, I'm confused how this patch is adding rereg_mr when it
already sort of seems to exist)
Jason
Lets present the facts first - Neither ibv_rereg_mr nor the kernel ABI
and command structure exist before this patch. The only thing that did
exist was an API between libibverbs and the provider library.
As there was no way for a userspace program to ask for memory
re-registration, AFAIK this op wasn't used by any provider.
Furthermore, re-registering a MR shouldn't change the user's MR handle!
Hence, there's no need to return information that the user already
knows. Does ibv_modify_qp return struct ibv_qp? Both verbs should act
similarly.
I don't mind re-spin this change into another patch, but since I don't
consider the current state of libibverbs as having memory
re-registration support, I don't think it's necessary.
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
