[ofa-general] Re: [PATCHv2 for-2.6.21] IB/mthca: QP reset fixes

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCHv2 for-2.6.21] IB/mthca: QP reset fixes
> 
> Yes, this definitely looks like a needed fix.  However:
> 
>  >  		if (qp->ibqp.send_cq != qp->ibqp.recv_cq)
>  >  			mthca_cq_clean(dev, to_mcq(qp->ibqp.send_cq), qp->qpn, NULL);
>  >  
>  > +		if (dev->mthca_flags & MTHCA_FLAG_MSI_X) {
>  > +			synchronize_irq(dev->eq_table.eq[MTHCA_EQ_COMP].msi_x_vector);
>  > +			synchronize_irq(dev->eq_table.eq[MTHCA_EQ_ASYNC].msi_x_vector);
>  > +		} else
>  > +			synchronize_irq(dev->pdev->irq);
> 
> I'm not quite sure I understand why we have to synchronize against the
> completion EQ's interrupt here, but I assume it's to make sure that no
> more CQEs are written that come from this QP's work queues.  Is that
> definitely necessary?  The hardware that writes CQEs isn't
> synchronized with the 2RST firmware command?
> 
> Anyway, assuming that we do need to synchronize with the completion
> interrupt, then the order looks suspicious above -- we clean out CQEs
> from CQs attached to the QP and then synchronize with the interrupt,
> which appears to leave a window where a new CQE could be written and
> not end up getting cleaned.
> 
> (and the same thing seems to apply to the changes to mthca_free_cq() also).

The hardware is synchronized, so it won't generate new events/CQEs after 2RST.
So I think in regular interrupt handler we do not need this synchronize_irq.
However, for MSI, interrupt handler for event could be running
on a CPU different from one servicing the command IFC, so  I think we need
the synchronize_irq there.

Makes sense?

-- 
MST