On 2020-05-19 05:43, Yamin Friedman wrote:
> + /*
> + * Allocated at least as many CQEs as requested, and otherwise
^^^^^^^^^
allocate?
> + spin_lock_irqsave(&dev->cq_pools_lock, flags);
> + list_splice(&tmp_list, &dev->cq_pools[poll_ctx - 1]);
> + spin_unlock_irqrestore(&dev->cq_pools_lock, flags);
Please add a WARN_ONCE() or WARN_ON_ONCE() statement that checks that
poll_ctx >= 1.
> +struct ib_cq *ib_cq_pool_get(struct ib_device *dev, unsigned int nr_cqe,
> + int comp_vector_hint,
> + enum ib_poll_context poll_ctx)
> +{
> + static unsigned int default_comp_vector;
> + int vector, ret, num_comp_vectors;
> + struct ib_cq *cq, *found = NULL;
> + unsigned long flags;
> +
> + if (poll_ctx > ARRAY_SIZE(dev->cq_pools) || poll_ctx == IB_POLL_DIRECT)
> + return ERR_PTR(-EINVAL);
How about changing this into the following?
if ((unsigned)(poll_ctx - 1) >= ARRAY_SIZE(dev->cq_pools))
return ...;
I think that change will make this code easier to verify.
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 1659131..d40604a 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -1555,6 +1555,7 @@ enum ib_poll_context {
> IB_POLL_SOFTIRQ, /* poll from softirq context */
> IB_POLL_WORKQUEUE, /* poll from workqueue */
> IB_POLL_UNBOUND_WORKQUEUE, /* poll from unbound workqueue */
> + IB_POLL_LAST,
> };
Please consider changing IB_POLL_LAST into IB_POLL_LAST =
IB_POLL_UNBOUND_WORKQUEUE. Otherwise the compiler will produce annoying
warnings on switch statements that do not handle IB_POLL_LAST explicitly.
Thanks,
Bart.
