[ofa-general] IPoIB CM (NOSRQ) [PATCH 1]

[Roland Dreier]

I can't find that much to object to here... the static table data
structure is pretty ugly but oh well.

I think with a few more fixes this could be mergable.  Anyone else
have an opinion?

Anyway:

 > +extern int max_rc_qp ;
 > +	index = id  & NOSRQ_INDEX_MASK ;
 > +		index = id  & NOSRQ_INDEX_MASK ;
 > +	index = (wc->wr_id & ~IPOIB_CM_OP_RECV) & NOSRQ_INDEX_MASK ;

since other fixes are needed please get rid of the spaces before
semicolons.

 > +module_param_named(nosrq_max_rc_qp, max_rc_qp, int, 0644);

Seems like making this writable is a bad idea -- I can't see how it
could possibly work if this changes after the driver starts.

 > +static void timer_check_srq(struct ipoib_dev_priv *priv, struct ipoib_cm_rx *p)
 > +{
 > +	unsigned long flags;
 > +
 > +	if (p && time_after_eq(jiffies, p->jiffies + IPOIB_CM_RX_UPDATE_TIME)) {
 > +		spin_lock_irqsave(&priv->lock, flags);
 > +		p->jiffies = jiffies;
 > +		/* Move this entry to list head, but do
 > +		 * not re-add it if it has been removed.
 > +		 */
 > +		if (p->state == IPOIB_CM_RX_LIVE)
 > +			list_move(&p->list, &priv->cm.passive_ids);
 > +		spin_unlock_irqrestore(&priv->lock, flags);
 > +	}
 > +}
 > +
 > +static void timer_check_nosrq(struct ipoib_dev_priv *priv, struct ipoib_cm_rx *p)
 > +{
 > +	unsigned long flags;
 > +
 > +	if (p && time_after_eq(jiffies, p->jiffies + IPOIB_CM_RX_UPDATE_TIME)) {
 > +		spin_lock_irqsave(&priv->lock, flags);
 > +		p->jiffies = jiffies;
 > +		/* Move this entry to list head, but do
 > +		 * not re-add it if it has been removed. */
 > +		if (!list_empty(&p->list))
 > +			list_move(&p->list, &priv->cm.passive_ids);
 > +		spin_unlock_irqrestore(&priv->lock, flags);
 > +	}
 > +}

I need to read more closely to understand this, but maybe you could
give a hint why you need two nearly identical copies of the same
function (which does not appear to be in the fast path)?

 - R.