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

Ralph Campbell
2005-12-19T20:50:36Z

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>