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

[Pradeep Satyanarayana]

Roland Dreier wrote:
> 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.

OK.

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

Yes, it should only be readable.

> 
>  > +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)?

These are in fact invoked by the receive handlers and that is why I had
them as two separate functions.

Pradeep