On Mon, May 25, 2020 at 08:14:23AM -0700, Bart Van Assche wrote:
> 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.
Yuk also.. It would be alot better to re-order IB_POLL_DIRECT to the
end of the enum and use a IB_POLL_LAST_POOL_TYPE to exclude it
directly.
Jason
