Fix potential deadlock in mthca

Here's a long-standing bug that lockdep found very nicely.

Ingo/Arjan, can you confirm that the fix looks OK and I am using spin_lock_nested() properly? I couldn't find much documentation or many examples of it, so I'm not positive this is the right way to handle this fix.

I'm inclined to put this fix in 2.6.18, since this is a kernel deadlock that is triggerable from userspace via uverbs. Comments?

Thanks,
Roland

commit a19aa5c5fdda8b556ab238177ee27c5ef7873c94

Author: Roland Dreier <[email protected]>
Date:   Fri Aug 11 08:56:57 2006 -0700

IB/mthca: Fix potential AB-BA deadlock with CQ locks

When destroying a QP, mthca locks both the QP's send CQ and receive CQ. However, the following scenario is perfectly valid:

QP_a: send_cq == CQ_x, recv_cq == CQ_y
QP_b: send_cq == CQ_y, recv_cq == CQ_x

The old mthca code simply locked send_cq and then recv_cq, which in this case could lead to an AB-BA deadlock if QP_a and QP_b were destroyed simultaneously.

We can fix this by changing the locking code to lock the CQ with the lower CQ number first, which will create a consistent lock ordering. Also, the second CQ is locked with spin_lock_nested() to tell lockdep that we know what we're doing with the lock nesting.

This bug was found by lockdep.

index 8de2887..9a5bece 100644
@@ -136,8 +136,8 @@ struct mthca_ah {
  * We have one global lock that protects dev->cq/qp_table.  Each
  * struct mthca_cq/qp also has its own lock.  An individual qp lock
  * may be taken inside of an individual cq lock.  Both cqs attached to
- * a qp may be locked, with the send cq locked first.  No other
- * nesting should be done.
+ * a qp may be locked, with the cq with the lower cqn locked first.
+ * No other nesting should be done.
  *
  * Each struct mthca_cq/qp also has an ref count, protected by the
  * corresponding table lock.  The pointer from the cq/qp_table to the

looks correct to me;

looks good to me too.

btw., we could introduce a new spin-lock op:

	spin_lock_double(l1, l2);
	...
	spin_unlock_double(l1, l2);

because some other code, like kernel/sched.c, fs/dcache.c and kernel/futex.c uses quite similar locking.

Ingo