Re: [openib-general] Re: [PATCH 10/13] [RFC] ipath verbs, part 1

[Ralph Campbell <ralphc@pathscale.com>]

The quick answer is the qp_list is traversed w/o the lock held in
ipath_ib_rcv().  The intent is to be able to do a lookup on the GID
to get a reference to the struct ipath_mcast and then walk the qp_list
w/o locks being held while processing the received packets at
interrupt level.


On Sun, 2005-12-18 at 11:59 -0800, Paul E. McKenney wrote:
> On Fri, Dec 16, 2005 at 03:48:55PM -0800, Roland Dreier wrote:
> > First half of ipath verbs driver
> 
> Some RCU-related questions interspersed.  Basic question is "where is
> the lock-free read-side traversal?"
> 
> 						Thanx, Paul
> 
> > ---
> > 
> >  drivers/infiniband/hw/ipath/ipath_verbs.c | 3244 +++++++++++++++++++++++++++++
> >  1 files changed, 3244 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/infiniband/hw/ipath/ipath_verbs.c
...
> > +/*
> > + * Insert the multicast GID into the table and
> > + * attach the QP structure.
> > + * Return zero if both were added.
> > + * Return EEXIST if the GID was already in the table but the QP was added.
> > + * Return ESRCH if the QP was already attached and neither structure was added.
> > + */
> > +static int ipath_mcast_add(struct ipath_mcast *mcast,
> > +			   struct ipath_mcast_qp *mqp)
> > +{
> > +	struct rb_node **n = &mcast_tree.rb_node;
> > +	struct rb_node *pn = NULL;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&mcast_lock, flags);
> > +
> > +	while (*n) {
> > +		struct ipath_mcast *tmcast;
> > +		struct ipath_mcast_qp *p;
> > +		int ret;
> > +
> > +		pn = *n;
> > +		tmcast = rb_entry(pn, struct ipath_mcast, rb_node);
> > +
> > +		ret = memcmp(mcast->mgid.raw, tmcast->mgid.raw,
> > +			     sizeof(union ib_gid));
> > +		if (ret < 0) {
> > +			n = &pn->rb_left;
> > +			continue;
> > +		}
> > +		if (ret > 0) {
> > +			n = &pn->rb_right;
> > +			continue;
> > +		}
> > +
> > +		/* Search the QP list to see if this is already there. */
> > +		list_for_each_entry_rcu(p, &tmcast->qp_list, list) {
> 
> Given that we hold the global mcast_lock, how is RCU helping here?

Its not really. I'm just trying to be consistent where ever the
qp_list is traversed.

> Is there a lock-free read-side traversal path somewhere that I am
> missing?

The lock free traversal is in ipath_ib_rcv() which is an interrupt
routine.

> > +			if (p->qp == mqp->qp) {
> > +				spin_unlock_irqrestore(&mcast_lock, flags);
> > +				return ESRCH;
> > +			}
> > +		}
> > +		list_add_tail_rcu(&mqp->list, &tmcast->qp_list);
> 
> Ditto...
> 
> > +		spin_unlock_irqrestore(&mcast_lock, flags);
> > +		return EEXIST;
> > +	}
> > +
> > +	list_add_tail_rcu(&mqp->list, &mcast->qp_list);
> 
> Ditto...
> 
> > +		spin_unlock_irqrestore(&mcast_lock, flags);
> > +
> > +	atomic_inc(&mcast->refcount);
> > +	rb_link_node(&mcast->rb_node, pn, n);
> > +	rb_insert_color(&mcast->rb_node, &mcast_tree);
> > +
> > +	spin_unlock_irqrestore(&mcast_lock, flags);
> > +
> > +	return 0;
> > +}

-- 
Ralph Campbell <ralphc@pathscale.com>