[ofw] patch: [ibal] use safe function for working on mutex.

Smith, Stan stan.smith at intel.com
Mon Oct 27 13:02:59 PDT 2008



________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Monday, October 27, 2008 10:21 AM
To: Tzachi Dar; ofw at lists.openfabrics.org
Subject: RE: [ofw] patch: [ibal] use safe function for working on mutex.

After some more experiments I have realized that working with mutex is not good enough as well since they tend to bring the user to APC_LEVEL.

Could you explain why the mutex is insufficient?
What is the behavior you see that makes you believe the mutex is not providing mutual exclusion?
At what code point is the IRQL @ dispatch and/or in an interrupt service routine?
PNP callbacks are in an AL thread context (passive).

thanks,

stan.

So I suggest to replace them to events.

The following patch was generated (but not tested yet):

Thanks
Tzachi


Index: bus_driver.c
===================================================================
--- bus_driver.c (revision 3372)
+++ bus_driver.c (working copy)
@@ -799,7 +799,7 @@
  // Mutex to synchronize multiple threads creating & deleting
  // control deviceobjects.

- ExInitializeFastMutex(&g_ControlMutex);
+ KeInitializeEvent(&g_ControlEvent, SynchronizationEvent, TRUE);
  g_bfi_InstanceCount = 0;
  memset(  __out_bcount(sizeof(g_bus_filters)) (void*)g_bus_filters, 0,
    sizeof(g_bus_filters) );
Index: bus_driver.h
===================================================================
--- bus_driver.h (revision 3372)
+++ bus_driver.h (working copy)
@@ -255,7 +255,7 @@

 extern bus_filter_t g_bus_filters[MAX_BUS_FILTERS];
 extern ULONG g_bfi_InstanceCount;
-extern FAST_MUTEX g_ControlMutex; // serializes InstanceCount & g_bus_filters
+extern KEVENT g_ControlEvent; // serializes InstanceCount & g_bus_filters

 extern bus_filter_t *alloc_bfi( IN DRIVER_OBJECT *, OUT int * );
 extern int free_bfi( IN bus_filter_t *p_bfi );
@@ -265,4 +265,12 @@
 extern bus_filter_t *get_set_bfi_by_ca_guid( IN net64_t ca_guid );
 extern char *get_obj_state_str(cl_state_t state);

+inline VOID lock_control_event() {
+ KeWaitForSingleObject(&g_ControlEvent, Executive, KernelMode , FALSE, NULL);
+}
+
+inline VOID unlock_control_event() {
+ KeSetEvent(&g_ControlEvent, 0, FALSE);
+}
+
 #endif /* !defined _BUS_DRIVER_H_ */
Index: bus_iou_mgr.c
===================================================================
--- bus_iou_mgr.c (revision 3373)
+++ bus_iou_mgr.c (working copy)
@@ -483,13 +483,13 @@
   */
  if ( !bus_globals.h_pnp_iou )
  {
-  ExAcquireFastMutex(&g_ControlMutex);
+  lock_control_event();
   if ( !bus_globals.h_pnp_iou )
   {
    bus_globals.h_pnp_iou = (ib_pnp_handle_t)1; /* block others */
    need_pnp_reg = TRUE;
   }
-  ExReleaseFastMutex(&g_ControlMutex);
+  unlock_control_event();

   if ( need_pnp_reg )
   {
Index: bus_pnp.c
===================================================================
--- bus_pnp.c (revision 3373)
+++ bus_pnp.c (working copy)
@@ -51,7 +51,7 @@
 static UNICODE_STRING al_ifc_name;
 static UNICODE_STRING ci_ifc_name;

-FAST_MUTEX    g_ControlMutex;
+KEVENT     g_ControlEvent;
 ULONG     g_bfi_InstanceCount;
 bus_filter_t   g_bus_filters[MAX_BUS_FILTERS];

@@ -334,7 +334,7 @@
   return status;
  }

- ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();
  if ( !gh_al ) {
   /* Initialize AL */
   ib_status = al_initialize();
@@ -343,12 +343,12 @@
    al_cleanup();
    BUS_TRACE_EXIT( BUS_DBG_ERROR, ("al_initialize returned %s.\n",
        ib_get_err_str(ib_status)) );
-   ExReleaseFastMutex(&g_ControlMutex);
+   unlock_control_event();
    return STATUS_UNSUCCESSFUL;
   }
   AL_init_here = TRUE;
  }
- ExReleaseFastMutex(&g_ControlMutex);
+ unlock_control_event();

  /* Initialize the port manager. */
  ib_status = create_port_mgr( p_ext->bus_filter, &p_ext->p_port_mgr );
@@ -1252,7 +1252,7 @@

  CL_ASSERT((obj_type == BFI_PORT_MGR_OBJ) || (obj_type == BFI_IOU_MGR_OBJ));

-    ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();

  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++) {

@@ -1272,7 +1272,7 @@
    }
   }
  }
- ExReleaseFastMutex(&g_ControlMutex);
+ unlock_control_event();

  BUS_PRINT( BUS_DBG_PNP,
     ("%s() cl_obj %p type %s_MGR_OBJ --> bfi[%d] %p\n",
@@ -1302,7 +1302,7 @@
   return matched;
  }

- ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();

  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
  {
@@ -1315,7 +1315,7 @@
    break;
   }
  }
- ExReleaseFastMutex(&g_ControlMutex);
+ unlock_control_event();

 #if DBG
  if ( !matched )
@@ -1376,7 +1376,7 @@
   */
  if ( !matched )
  {
-  ExAcquireFastMutex(&g_ControlMutex);
+  lock_control_event();

   for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
   {
@@ -1391,7 +1391,7 @@
     break;
    }
   }
-  ExReleaseFastMutex(&g_ControlMutex);
+  unlock_control_event();
  }

  BUS_PRINT( BUS_DBG_PNP,
@@ -1413,7 +1413,7 @@
      * IoCreateDeviceSecure & IoCreateSymbolicLink must be called at
      * PASSIVE_LEVEL.
   */
- ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();

  // find 1st unused bfi slot.
  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
@@ -1430,7 +1430,7 @@
    break;
   }
  }
- ExReleaseFastMutex(&g_ControlMutex);
+ unlock_control_event();

 #if DBG
  RtlStringCbPrintfA ( p_bfi->whoami,
@@ -1453,12 +1453,12 @@
 {
  int remaining;

- ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();
  p_bfi->p_bus_ext = NULL;
  p_bfi->ca_guid = 0ULL;
  remaining = --g_bfi_InstanceCount; // one less bfi in-use
- ExReleaseFastMutex(&g_ControlMutex);
-
+ unlock_control_event();
+
  return remaining;
 }

@@ -1467,10 +1467,10 @@
 {
  int ic;

- ExAcquireFastMutex(&g_ControlMutex);
+ lock_control_event();
  ic = g_bfi_InstanceCount;
- ExReleaseFastMutex(&g_ControlMutex);
-
+ unlock_control_event();
+
  return ic;
 }

Index: bus_port_mgr.c
===================================================================
--- bus_port_mgr.c (revision 3373)
+++ bus_port_mgr.c (working copy)
@@ -483,12 +483,12 @@
   */
  if ( !bus_globals.h_pnp_port )
  {
-  ExAcquireFastMutex(&g_ControlMutex);
+  lock_control_event();
   if ( !bus_globals.h_pnp_port ) {
    bus_globals.h_pnp_port = (ib_pnp_handle_t)1; /* block others */
    need_pnp_reg = TRUE;
   }
-  ExReleaseFastMutex(&g_ControlMutex);
+  unlock_control_event();

   if ( need_pnp_reg )
   {
@@ -1005,10 +1005,10 @@
   if ( !p_bfi->p_bus_ext )
    continue;
   GO = FALSE;
-  ExAcquireFastMutex(&g_ControlMutex);
+  lock_control_event();
   if ( p_bfi->ca_guid && p_bfi->p_port_mgr )
    GO = TRUE;
-  ExReleaseFastMutex(&g_ControlMutex);
+  unlock_control_event();
   if ( GO == FALSE )
    continue;
   status = _port_mgr_pkey_rem( pkeys, p_bfi->p_port_mgr );
@@ -1149,10 +1149,10 @@
   if ( !p_bfi->p_bus_ext )
    continue;
   GO = FALSE;
-  ExAcquireFastMutex(&g_ControlMutex);
+  lock_control_event();
   if ( p_bfi->ca_guid && p_bfi->p_port_mgr )
    GO = TRUE;
-  ExReleaseFastMutex(&g_ControlMutex);
+  unlock_control_event();
   if ( GO == FALSE )
    continue;
   status = _port_mgr_pkey_add( pkeys, p_bfi, p_bfi->p_port_mgr );

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Monday, October 27, 2008 4:58 PM
To: ofw at lists.openfabrics.org
Subject: [ofw] patch: [ibal] use safe function for working on mutex.

The unsafe functions can only be used from apc level. We call them from passive level, so we use the safe functions.

Index: Q:/projinf3/trunk/core/bus/kernel/bus_port_mgr.c
===================================================================
--- Q:/projinf3/trunk/core/bus/kernel/bus_port_mgr.c (revision 3372)
+++ Q:/projinf3/trunk/core/bus/kernel/bus_port_mgr.c (revision 3373)
@@ -483,12 +483,12 @@
   */
  if ( !bus_globals.h_pnp_port )
  {
-  ExAcquireFastMutexUnsafe(&g_ControlMutex);
+  ExAcquireFastMutex(&g_ControlMutex);
   if ( !bus_globals.h_pnp_port ) {
    bus_globals.h_pnp_port = (ib_pnp_handle_t)1; /* block others */
    need_pnp_reg = TRUE;
   }
-  ExReleaseFastMutexUnsafe(&g_ControlMutex);
+  ExReleaseFastMutex(&g_ControlMutex);

   if ( need_pnp_reg )
   {
@@ -1005,10 +1005,10 @@
   if ( !p_bfi->p_bus_ext )
    continue;
   GO = FALSE;
-  ExAcquireFastMutexUnsafe(&g_ControlMutex);
+  ExAcquireFastMutex(&g_ControlMutex);
   if ( p_bfi->ca_guid && p_bfi->p_port_mgr )
    GO = TRUE;
-  ExReleaseFastMutexUnsafe(&g_ControlMutex);
+  ExReleaseFastMutex(&g_ControlMutex);
   if ( GO == FALSE )
    continue;
   status = _port_mgr_pkey_rem( pkeys, p_bfi->p_port_mgr );
@@ -1149,10 +1149,10 @@
   if ( !p_bfi->p_bus_ext )
    continue;
   GO = FALSE;
-  ExAcquireFastMutexUnsafe(&g_ControlMutex);
+  ExAcquireFastMutex(&g_ControlMutex);
   if ( p_bfi->ca_guid && p_bfi->p_port_mgr )
    GO = TRUE;
-  ExReleaseFastMutexUnsafe(&g_ControlMutex);
+  ExReleaseFastMutex(&g_ControlMutex);
   if ( GO == FALSE )
    continue;
   status = _port_mgr_pkey_add( pkeys, p_bfi, p_bfi->p_port_mgr );
Index: Q:/projinf3/trunk/core/bus/kernel/bus_pnp.c
===================================================================
--- Q:/projinf3/trunk/core/bus/kernel/bus_pnp.c (revision 3372)
+++ Q:/projinf3/trunk/core/bus/kernel/bus_pnp.c (revision 3373)
@@ -334,7 +334,7 @@
   return status;
  }

- ExAcquireFastMutexUnsafe(&g_ControlMutex);
+ ExAcquireFastMutex(&g_ControlMutex);
  if ( !gh_al ) {
   /* Initialize AL */
   ib_status = al_initialize();
@@ -343,12 +343,12 @@
    al_cleanup();
    BUS_TRACE_EXIT( BUS_DBG_ERROR, ("al_initialize returned %s.\n",
        ib_get_err_str(ib_status)) );
-   ExReleaseFastMutexUnsafe(&g_ControlMutex);
+   ExReleaseFastMutex(&g_ControlMutex);
    return STATUS_UNSUCCESSFUL;
   }
   AL_init_here = TRUE;
  }
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

  /* Initialize the port manager. */
  ib_status = create_port_mgr( p_ext->bus_filter, &p_ext->p_port_mgr );
@@ -1252,7 +1252,7 @@

  CL_ASSERT((obj_type == BFI_PORT_MGR_OBJ) || (obj_type == BFI_IOU_MGR_OBJ));

-    ExAcquireFastMutexUnsafe(&g_ControlMutex);
+    ExAcquireFastMutex(&g_ControlMutex);

  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++) {

@@ -1272,7 +1272,7 @@
    }
   }
  }
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

  BUS_PRINT( BUS_DBG_PNP,
     ("%s() cl_obj %p type %s_MGR_OBJ --> bfi[%d] %p\n",
@@ -1302,7 +1302,7 @@
   return matched;
  }

- ExAcquireFastMutexUnsafe(&g_ControlMutex);
+ ExAcquireFastMutex(&g_ControlMutex);

  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
  {
@@ -1315,7 +1315,7 @@
    break;
   }
  }
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

 #if DBG
  if ( !matched )
@@ -1376,7 +1376,7 @@
   */
  if ( !matched )
  {
-  ExAcquireFastMutexUnsafe(&g_ControlMutex);
+  ExAcquireFastMutex(&g_ControlMutex);

   for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
   {
@@ -1391,7 +1391,7 @@
     break;
    }
   }
-  ExReleaseFastMutexUnsafe(&g_ControlMutex);
+  ExReleaseFastMutex(&g_ControlMutex);
  }

  BUS_PRINT( BUS_DBG_PNP,
@@ -1413,7 +1413,7 @@
      * IoCreateDeviceSecure & IoCreateSymbolicLink must be called at
      * PASSIVE_LEVEL.
   */
- ExAcquireFastMutexUnsafe(&g_ControlMutex);
+ ExAcquireFastMutex(&g_ControlMutex);

  // find 1st unused bfi slot.
  for(p_bfi=g_bus_filters; p_bfi < &g_bus_filters[MAX_BUS_FILTERS]; p_bfi++)
@@ -1430,7 +1430,7 @@
    break;
   }
  }
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

 #if DBG
  RtlStringCbPrintfA ( p_bfi->whoami,
@@ -1453,11 +1453,11 @@
 {
  int remaining;

- ExAcquireFastMutexUnsafe(&g_ControlMutex);
+ ExAcquireFastMutex(&g_ControlMutex);
  p_bfi->p_bus_ext = NULL;
  p_bfi->ca_guid = 0ULL;
  remaining = --g_bfi_InstanceCount; // one less bfi in-use
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

  return remaining;
 }
@@ -1467,9 +1467,9 @@
 {
  int ic;

- ExAcquireFastMutexUnsafe(&g_ControlMutex);
+ ExAcquireFastMutex(&g_ControlMutex);
  ic = g_bfi_InstanceCount;
- ExReleaseFastMutexUnsafe(&g_ControlMutex);
+ ExReleaseFastMutex(&g_ControlMutex);

  return ic;
 }
Index: Q:/projinf3/trunk/core/bus/kernel/bus_iou_mgr.c
===================================================================
--- Q:/projinf3/trunk/core/bus/kernel/bus_iou_mgr.c (revision 3372)
+++ Q:/projinf3/trunk/core/bus/kernel/bus_iou_mgr.c (revision 3373)
@@ -483,13 +483,13 @@
   */
  if ( !bus_globals.h_pnp_iou )
  {
-  ExAcquireFastMutexUnsafe(&g_ControlMutex);
+  ExAcquireFastMutex(&g_ControlMutex);
   if ( !bus_globals.h_pnp_iou )
   {
    bus_globals.h_pnp_iou = (ib_pnp_handle_t)1; /* block others */
    need_pnp_reg = TRUE;
   }
-  ExReleaseFastMutexUnsafe(&g_ControlMutex);
+  ExReleaseFastMutex(&g_ControlMutex);

   if ( need_pnp_reg )
   {

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20081027/e790dea5/attachment.html>


More information about the ofw mailing list