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

[Leonid Keller]

I think this patch should go also into the branch.
I'll be on vacation 08/19-26, so i'll appreciate a lot if you could look
it through and opine.
Also if someone could check it, running WHQL tests ...
 
There are two Remove Locks in IBBUS code. They are called 'remove_lock'
and 'stop_lock'.
The 'stop_lock' get initialized twice, on device' start and stop.
The 'remove_lock' is initialized only once on the start up.
In my patch I did the same with 'stop_lock'.
But honestly, I don't quite understand why the author of the code didn't
do this in the first place ...

________________________________

From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Leonid Keller
Sent: Sunday, August 16, 2009 7:46 PM
To: ofw at lists.openfabrics.org
Subject: [ofw][patch][IBBUS] crash on re-initialization of remove lock


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/20090817/e63cdae8/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT9869809.txt
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20090817/e63cdae8/attachment.txt>