> +t3c_cpl_handler_t t3c_handlers[NUM_CPL_CMDS];
Can we kill the _t suffix here? I had to search through the code to
see that this is a function type, so it would be better to call this
something like t3c_cpl_handler_func.
> +DECLARE_MUTEX(dev_mutex);
static DEFINE_MUTEX() instead? (ie make it static since it's only
used in this function, and make it a real mutex_lock/mutex_unlock
mutex instead of a (deprecated) semaphore).
> + if (!(rnicp->pdid2hlp = kzalloc(sizeof(void *) * T3_MAX_NUM_PD,
> + GFP_KERNEL)))
> + goto pdid_err;
kernel idiom is to do
rnicp->pdid2hlp = kzalloc(sizeof(void *) * T3_MAX_NUM_PD, GFP_KERNEL);
if (!rnicp->pdid2hlp)
goto pdid_err;
it's the same number of lines and it's easier to read.
> + rnicp->attr.maxPDs = T3_MAX_NUM_PD - 1;
> + rnicp->attr.memPgSizesBitMask = 0x7FFF; /* 4KB-128MB */
> + rnicp->attr.canResizeWQ = 0;
> + rnicp->attr.maxRDMAreadsPerQP = 8;
> + rnicp->attr.maxRDMAreadResources =
> + rnicp->attr.maxRDMAreadsPerQP * rnicp->attr.maxQPs;
> + rnicp->attr.maxRDMAreadQPdepth = 8; /* IRD */
> + rnicp->attr.maxRDMAreaddepth =
> + rnicp->attr.maxRDMAreadQPdepth * rnicp->attr.maxQPs;
Please switch from StudlyCAPS to underscore_between_words style for
these member names.
> + list_for_each_entry(dev, &dev_list, entry) {
> + if (dev->rdev.t3cdev_p == tdev) {
> + list_del(&dev->entry);
> + iwch_unregister_device(dev);
> + cxio_rdev_close(&dev->rdev);
> + kfree(dev->pdid2hlp);
> + kfree(dev->cqid2hlp);
> + kfree(dev->stag2hlp);
> + kfree(dev->qpid2hlp);
> + ib_dealloc_device(&dev->ibdev);
This needs to be list_for_each_entry_safe(), because
ib_dealloc_device() is freeing dev and then list_for_each_entry
dereferences it the next time through the loop.
- R.
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
