[ewg] [PATCH]mlx4_ib XRC RCV: Fix mlx4_ib_reg_xrc_rcv_qp() locking

[sebastien dugue]

On Wed, 16 Feb 2011 14:50:02 +0200
Jack Morgenstein <jackm at 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.dugue at 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);
> > 
>