[ewg] [PATCH 1/9] [RFC] Adds the Xsigo unified API for IB and CM access used by the Xsigo virtual (v*) drivers like vnic and vhba

[Roland Dreier]

 > This patch adds the Xsigo unified API for IB and CM access used by the
 > Xsigo virtual (v*) drivers like vnic and vhba.

what's wrong with the verbs/CM API we already have?

 > +static inline struct xsigo_ib_connect_info *get_connect_info(int handle)

What's the reasoning behind using handles that have to be looked up all
the time?  Seems like the code would be simpler and faster if you just
used pointers to a structure directly.

 > +void xsigo_ib_disconnect(u32 handle)
 > +{
 > +	struct xsigo_ib_connect_info *connect_info;
 > +
 > +	if (handle == XSIGO_IB_ERROR_HANDLE)
 > +		goto xsigo_ib_disconnect_exit2;
 > +
 > +	connect_info = get_connect_info(handle);
 > +
 > +	if (!connect_info->used)
 > +		goto xsigo_ib_disconnect_exit;
 > +
 > +	xsigoib_debug(KERN_INFO, "xsigo_ib_disconnect called for handle %d\n",
 > +		      handle);
 > +
 > +	connect_info->active = 0; /* Don't make any more calls to this handle */
 > +	wait_event_timeout(xsigoib_wait, !atomic_read(&connect_info->refcount),
 > +			   5 * HZ);

reference counting looks very racy... seems like there are a lot of ways
for this disconnect code to get past the wait_event here (and shouldn't
you do something if it times out??) and then have another CPU bump the
refcount too late.

 > +void xsigoib_exit(void)

 > +	mdelay(2000);

very suspicious-looking.  Is this papering over something?

 - R.