On Mon, 2006-03-06 at 14:28 -0800, Roland Dreier wrote:
> > +t3c_cpl_handler_t t3c_handlers[NUM_CPL_CMDS];
>
ok.
> 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).
>
Ok. I'll change all semaphores to mutex.
> > + 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.
>
agreed. I'll fix this everywhere.
> > + 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.
>
SorryAboutThat. will_change.
:-)
> > + 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.
>
Yup!
Is it SOP to post the resulting patch for these changes?
Steve.
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
