Re: [PATCH libibverbs v6 3/7] livibverbs: Add support for XRC SRQs

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Tue, Jun 04, 2013 at 11:49:47AM -0700, sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:

>  enum verbs_context_mask {
>  	VERBS_CONTEXT_XRCD	= 1 << 0,
> -	VERBS_CONTEXT_RESERVED	= 1 << 1
> +	VERBS_CONTEXT_SRQ	= 1 << 1,
> +	VERBS_CONTEXT_RESERVED	= 1 << 2
>  };

Why is _RESERVED being re-numbered here? That worries me..

For that matter, what is it for?

Frankly, I would ditch mask for verbs_context op members and other
structures that are size based. What does it mean if the size includes
get_srq_num, but VERBS_CONTEXT_SRQ is 0 and *get_srq_num is
null/non-null?

Simpler rule: If the size says the entry is there, then it is
there.

Reserve mask for some other future use.

> +static inline uint32_t ibv_get_srq_num(struct ibv_srq *srq)
> +{
> +	struct verbs_context *vctx = verbs_get_ctx_op(srq->context, get_srq_num);
> +	if (!vctx) {
> +		errno = ENOSYS;
> +		return 0;
> +	}
> +	return vctx->get_srq_num(srq);

Might want to stream line this flow a bit:

static inline struct verbs_context *__verbs_get_ctx_op(struct ibv_context *context,
                                                       size_t op)
{
       struct verbs_context *vctx = verbs_get_ctx(ctx);
       if (vctx && vctx->sz >= sizeof(*vctx) - op &&
           *((void **)vctx + op) != NULL)
         return vctx;
       errno = ENOSYS;
       return NULL;
}
#define verbs_get_ctx_op(ctx, op) __verbs_get_ctx_op(ctx, offsetof(struct verbs_context, op))

static inline uint32_t ibv_get_srq_num(struct ibv_srq *srq)
{
	struct verbs_context *vctx = verbs_get_ctx_op(srq->context,get_srq_num);
	if (!vctx)
	     return 0;
	return vctx->get_srq_num(srq);
}

> diff --git a/src/libibverbs.map b/src/libibverbs.map
> index c190eb9..00f9062 100644
> +++ b/src/libibverbs.map
> @@ -103,4 +103,6 @@ IBVERBS_1.1 {
>  
>  		ibv_cmd_open_xrcd;
>  		ibv_cmd_close_xrcd;
> +		ibv_cmd_create_srq_ex;
> +		
>  } IBVERBS_1.0;

Hum, so drivers that implement XRC will need to be paired with a new
verbs... Bit disappointing, did we want to try and address that?

Jason
--
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