On Wed, 16 Feb 2011 14:50:02 +0200
Jack Morgenstein <ja...@dev.mellanox.co.il> wrote:
> You are correct! Good catch.
> We will add this to OFED.
Thanks,
>
> (P.S., I would rather leave irqsave -- it is used everywhere else for this
> spinlock).
Right, but everywhere you know for sure you're in which context you are
(process
or interrupt), there's no need to use the save/restore variant. Those are just
to be
used in places where you don't know in which context you are.
Also, one thing I noticed in that same function: why allocate ctx_entry before
knowing if it's going to be of any use? The allocation could be done right
before
the first use.
Sébastien.
>
> -Jack
>
> On Monday 14 February 2011 09:32, sebastien dugue wrote:
> >
> > Resending to the proper ML (sorry).
> >
> >
> > In mlx4_ib_reg_xrc_rcv_qp(), we need to take the xrc_reg_list_lock
> > spinlock
> > when walking the xrc_reg_list.
> >
> > We've been hit by this on 2 customer sites.
> >
> > Also, I guess spin_lock_irqsave() could be replaced by spin_lock_irq() in
> > that function as we know for sure we're in process context.
> >
> > Signed-off-by: Sébastien Dugué <sebastien.du...@bull.net>
> >
> > --
> >
> > qp.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > dIndex: kernel-ib/drivers/infiniband/hw/mlx4/qp.c
> > ===================================================================
> > --- kernel-ib.orig/drivers/infiniband/hw/mlx4/qp.c 2011-01-31
> > 16:52:11.000000000 +0100
> > +++ kernel-ib/drivers/infiniband/hw/mlx4/qp.c 2011-02-11
> > 15:24:27.000000000 +0100
> > @@ -2549,13 +2549,16 @@
> > }
> >
> > mutex_lock(&mibqp->mutex);
> > + spin_lock_irqsave(&mibqp->xrc_reg_list_lock, flags);
> > list_for_each_entry(tmp, &mibqp->xrc_reg_list, list)
> > if (tmp->context == context) {
> > + spin_unlock_irqrestore(&mibqp->xrc_reg_list_lock,
> > flags);
> > mutex_unlock(&mibqp->mutex);
> > kfree(ctx_entry);
> > mutex_unlock(&to_mdev(xrcd->device)->xrc_reg_mutex);
> > return 0;
> > }
> > + spin_unlock_irqrestore(&mibqp->xrc_reg_list_lock, flags);
> >
> > ctx_entry->context = context;
> > spin_lock_irqsave(&mibqp->xrc_reg_list_lock, flags);
> >
>
_______________________________________________
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
