[ofw][patch][IBBUS] crash on re-initialization of remove lock

[Leonid Keller]

I was reported a crash upon running "System Common Scenario" WHQL test
with our stack.
The crash: C4 (0xd7), which means Driver Verifier revealed a
re-initializing of Remove Lock.
 
fffff880`01fa73a8 fffff800`0191b3dc : nt!KeBugCheckEx
fffff880`01fa73b0 fffff800`0192d2e6 : nt!NtShutdownSystem+0x7f0c
fffff880`01fa73f0 fffff880`02a06974 : nt!NtShutdownSystem+0x19e16
fffff880`01fa7450 fffff800`01937c16 : ibbus!cl_pnp+0x1d8
[f:\ribel\mlnx_winof_2.0.5\4335\branches\mlnx_winof_2-0\core\complib\ker
nel\cl_pnp_po.c @ 211]
fffff880`01fa74b0 fffff800`0193a52a : nt!NtShutdownSystem+0x24746
fffff880`01fa7510 fffff800`01937c16 : nt!NtShutdownSystem+0x2705a

 
It happens on win2k8 R2.
One can see that IBAL PnP code, written in 2002 (?) contains in fact
re-initialization of stop Remove  Lock.
Maybe sometimes it was possible and was not checked by Driver Verifier
or we just haven't come accross it...
 
Here is a patch that fixes the problem by eliminating re-initialization
of the stop lock.
 
 
Index: core/bus/kernel/bus_pnp.c
===================================================================
--- core/bus/kernel/bus_pnp.c (revision 4659)
+++ core/bus/kernel/bus_pnp.c (working copy)
@@ -216,7 +216,6 @@
    RtlZeroMemory(p_ext, sizeof *p_ext);
    cl_init_pnp_po_ext( g_ControlDeviceObject, NULL, 
         NULL, bus_globals.dbg_lvl, NULL, NULL );
-   IoInitializeRemoveLock( &p_ext->cl_ext.stop_lock, 'dtci', 0, 1000 );
 
    /* enable user-mode access to IB stack */
    BUS_PRINT( BUS_DBG_PNP, ("Remove-n-reCreate dos_name symlink\n") );
Index: core/complib/kernel/cl_pnp_po.c
===================================================================
--- core/complib/kernel/cl_pnp_po.c (revision 4659)
+++ core/complib/kernel/cl_pnp_po.c (working copy)
@@ -137,7 +137,8 @@
  /* Store the pointer to our own device. */
  p_ext->p_self_do = p_dev_obj;
  IoInitializeRemoveLock( &p_ext->remove_lock, 'bilc', 15, 1000 );
-
+ IoInitializeRemoveLock( &p_ext->stop_lock, 'dtci', 0, 1000 );
+ 
  /* Initialize the PnP states. */
  p_ext->pnp_state = NotStarted;
  p_ext->last_pnp_state = NotStarted;
@@ -428,15 +429,6 @@
  if( NT_SUCCESS( status ) )
   cl_set_pnp_state( p_ext, Started );
 
- /*
-  * If we get the start request when we're already started, don't 
-  * re-initialize the stop lock.
-  */
- if( p_ext->last_pnp_state != Started ) {
-  CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl, ("IoInitializeRemoveLock:
stop_lock %p[\n", &p_ext->stop_lock));
-  IoInitializeRemoveLock( &p_ext->stop_lock, 'dtci', 0, 1000 );
- }
-
  CL_EXIT( CL_DBG_PNP, p_ext->dbg_lvl );
  return status;
 }
@@ -557,12 +549,6 @@
 
  if( p_ext->last_pnp_state == Started )
  {
-  /*
-   * Re-initialize the stop lock before rolling back the PnP
-   * state so that there's no contention while it's uninitialized.
-   */
-  CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl, ("IoInitializeRemoveLock:
stop_lock %p[\n", &p_ext->stop_lock));
-  IoInitializeRemoveLock( &p_ext->stop_lock, 'dtci', 0, 1000 );
 #if 0  
   // leo: it seems like a bug, because it can never get released
   {

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