[ofw] [Patch 3/3 ][ALL] Moving "shutter.h" under different location

[Tzachi Dar]

Two comments:

1) The patch seems to refer to ipob_port.cpp and not to ipoib_adapter.cpp
2) on the function __mcast_cb you call the function pending_list_destroy but without taking the send_lock first.

Thanks
Tzachi

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Monday, January 25, 2010 8:50 PM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: [ofw] [Patch 3/3 ][ALL] Moving "shutter.h" under different location

This patches orders the spinlocks to avoid deadlock.
Send lock can't be invoked after port object was locked.
It creates classic dedlock:
Thread 1: wait(A); wait(B)
Thread 2: wait(B); wait(A)

Signed-off by: Alexander Naslednikov (xalex at mellanox.com),
      Tzachi Dar (tzachid at mellanox.com)
Index: ipoib_adapter.cpp
===================================================================
--- ipoib_adapter.cpp (revision 5452)
+++ ipoib_adapter.cpp (working copy)
@@ -3243,14 +3294,8 @@
 {
  cl_list_item_t  *p_item;
  ipoib_send_NB_SG  *s_buf;
- ULONG    send_complete_flags = 0;
+ ULONG    send_complete_flags = NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL;

- if (KeGetCurrentIrql() == DISPATCH_LEVEL)
- {
-  NDIS_SET_SEND_COMPLETE_FLAG(send_complete_flags, NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL);
- }
-
- cl_spinlock_acquire( &p_port->send_lock );
  /* Complete any pending packets. */
  for( p_item = cl_qlist_remove_head( &p_port->send_mgr.pending_list );
   p_item != cl_qlist_end( &p_port->send_mgr.pending_list );
@@ -3260,13 +3305,10 @@
   ASSERT(s_buf->p_port == p_port);
   ASSERT(s_buf->p_nbl);

-
+  //TODO
   //__send_complete_net_buffer(s_buf, NDIS_STATUS_RESET_IN_PROGRESS,send_complete_flags,TRUE);
   __send_complete_net_buffer(s_buf, NDIS_STATUS_FAILURE,send_complete_flags,TRUE);
  }
-
-
- cl_spinlock_release( &p_port->send_lock );

 }

@@ -3278,7 +3320,9 @@
  //Destroy pending list and put all the send buffers back to pool
  //The list should be already destroyed at this point
  ASSERT(p_port->send_mgr.pending_list.count == 0);
+ cl_spinlock_acquire( &p_port->send_lock );
  __pending_list_destroy(p_port);
+ cl_spinlock_release( &p_port->send_lock );

  // Now, destroy the send pool
  cl_qpool_destroy(&p_port->send_mgr.send_pool);
@@ -5299,7 +5343,6 @@
   //TODO Tzachid: make an assert here to validate your IRQL
   ASSERT (KeGetCurrentIrql() == DISPATCH_LEVEL);
   old_irql = DISPATCH_LEVEL;
-  NDIS_SET_SEND_COMPLETE_FLAG(send_complete_flags, NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL);
  } else {
   NDIS_RAISE_IRQL_TO_DISPATCH(&old_irql);
   //ASSERT (KeGetCurrentIrql() == PASSIVE_LEVEL); // Happens
@@ -7158,6 +7201,7 @@
   * object lock since that is the order taken when reposting.
   */
  cl_spinlock_acquire( &p_port->recv_lock );
+ cl_spinlock_acquire( &p_port->send_lock );
  cl_obj_lock( &p_port->obj );
  p_port->state = IB_QPS_ERROR;

@@ -7172,6 +7216,7 @@
   p_port->ib_mgr.h_query = NULL;
  }
  cl_obj_unlock( &p_port->obj );
+ cl_spinlock_release( &p_port->send_lock );
  cl_spinlock_release( &p_port->recv_lock );

  KeWaitForSingleObject(
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20100126/0500f3cf/attachment.html>