On Tuesday 18 May 2010 21:33, Sean Hefty wrote:
> >+struct ib_xrc_rcv_qp_table_entry *
> >+ib_xrc_rcv_tbl_find(struct ib_device *dev, u32 qpn)
> >+{
> >+ return radix_tree_lookup(&dev->xrc_rcv_qp_table, qpn);
> >+}
>
> nit - but do we need a wrapper around this single call?
I prefer it this way, so that we have a complete xrc_rcv_table interface.
> >+ list_for_each_entry(tmp, &qp->list, list)
> >+ if (tmp->context == context) {
> >+ found = 1;
> >+ break;
> >+ }
> >+ /* add only a single entry per user context */
> >+ if (unlikely(found)) {
> >+ err = 0;
> >+ goto free_out;
> >+ }
>
> Maybe this becomes clear later, but can a user add multiple entries? Can't we
> just consider this a usage error? Actually, why does it matter if the same
> context is added multiple times?
This list of entries is also used to disperse XRC RCV qp events to all
registered
processes -- and we only want a single event dispatched per process.
> >+ dev->destroy_xrc_rcv_qp(qp->xrcd, qpn);
> >+ kfree(qp);
> >+ list_for_each_entry_safe(reg_entry, tmp, &xrc_local, list) {
> >+ list_del(®_entry->list);
> >+ kfree(reg_entry);
> >+ atomic_dec(&qp->xrcd->usecnt);
>
> qp was just freed a few lines up
Ouch! Thanks for the catch, Sean. I'll fix it before resubmitting.
Regards,
Jack
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
