[openib-general] [PATCH][RFC] uverbs SRQ implementation

[Grant Grundler]

On Wed, Aug 03, 2005 at 09:28:04AM -0700, Roland Dreier wrote:
> Feedback in the meantime appreciated, though...
...
> +	if (cmd.is_srq)
> +		srq = idr_find(&ib_uverbs_srq_idr, cmd.srq_handle);
> +	else
> +		srq = NULL;

my preference is to write this as:
	srq = cmd.is_srq ? idr_find(&ib_uverbs_srq_idr, cmd.srq_handle) : NULL;

>  	if (!pd  || pd->uobject->context  != file->ucontext ||
>  	    !scq || scq->uobject->context != file->ucontext ||
> -	    !rcq || rcq->uobject->context != file->ucontext) {
> +	    !rcq || rcq->uobject->context != file->ucontext ||
> +	    (cmd.is_srq && (!srq || srq->uobject->context != file->ucontext))) {

I think it's redudant to test cmd.is_srq.
srq is NULL if cmd.is_srq is not set.
ie !srq should short circuit the rest of the test.

if idr_find() fails, I would expect it to return NULL.

...
> +ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
> +			     const char __user *buf, int in_len,
> +			     int out_len)
> +{
...

> +retry:
> +	if (!idr_pre_get(&ib_uverbs_srq_idr, GFP_KERNEL)) {
> +		ret = -ENOMEM;
> +		goto err_destroy;
> +	}
> +
> +	ret = idr_get_new(&ib_uverbs_srq_idr, srq, &uobj->id);
> +
> +	if (ret == -EAGAIN)
> +		goto retry;

Do I need to worry about infinite (or very long) retry loops here?
If not, maybe add a one-liner comment explaining what limits the retry.

I'm not clueful enough to know if the rest is correct or not.

hth,
grant