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
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
