[openib-general] [PATCH 02/18] [RFC] Provider Device Discovery

[Roland Dreier]

 > +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.