<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META http-equiv=Content-Type content="text/html; charset=us-ascii">
<META content="MSHTML 6.00.2900.5512" name=GENERATOR></HEAD>
<BODY dir=ltr>
<DIV><SPAN class=906325615-02092009><FONT face=Arial color=#0000ff size=2>Thank
you. Go ahead.</FONT></SPAN></DIV><BR>
<BLOCKQUOTE dir=ltr
style="PADDING-LEFT: 5px; MARGIN-LEFT: 5px; BORDER-LEFT: #0000ff 2px solid; MARGIN-RIGHT: 0px">
<DIV class=OutlookMessageHeader lang=en-us dir=ltr align=left>
<HR tabIndex=-1>
<FONT face=Tahoma size=2><B>From:</B> Stan C. Smith
[mailto:stan.smith@intel.com] <BR><B>Sent:</B> Wednesday, September 02, 2009
6:50 PM<BR><B>To:</B> Leonid Keller; Fab Tillier; Hefty, Sean<BR><B>Cc:</B>
ofw_list<BR><B>Subject:</B> RE: [ofw][patch][IBBUS] crash on re-initialization
of remove lock<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>Leo,</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2> I've done some testing of your suggested patch
with WHQL enable/disable tests + standard WinOF load/unload and seen no
problems.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>Enclosed are the patches which I suggested to you in
previous email - specifically remove the empty if block.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>On your approval I will commit to branch and
trunk.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>stan.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>--- C:/Documents and Settings/scsmith/Local
Settings/Temp/bus_pnp.c-revBASE.svn000.tmp.c Wed Sep 02 08:44:56
2009<BR>+++ C:/Documents and Settings/scsmith/My
Documents/openIB-windows/SVN/gen1/branches/WOF2-1/core/bus/kernel/bus_pnp.c Fri
Aug 28 16:35:21 2009<BR>@@ -216,7 +216,6
@@<BR> RtlZeroMemory(p_ext, sizeof
*p_ext);<BR> cl_init_pnp_po_ext( g_ControlDeviceObject,
NULL, <BR> NULL,
bus_globals.dbg_lvl, NULL, NULL
);<BR>- IoInitializeRemoveLock(
&p_ext->cl_ext.stop_lock, 'dtci', 0, 1000
);<BR> <BR> /* enable user-mode access to IB stack
*/<BR> BUS_PRINT( BUS_DBG_PNP, ("Remove-n-reCreate
dos_name symlink\n") );<BR></FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2>--- C:/Documents and Settings/scsmith/Local
Settings/Temp/cl_pnp_po.c-revBASE.svn000.tmp.c Wed Sep 02 08:43:28
2009<BR>+++ C:/Documents and Settings/scsmith/My
Documents/openIB-windows/SVN/gen1/branches/WOF2-1/core/complib/kernel/cl_pnp_po.c Fri
Aug 28 16:36:18 2009<BR>@@ -136,7 +136,9 @@<BR> <BR> /* Store
the pointer to our own device. */<BR> p_ext->p_self_do =
p_dev_obj;<BR>+<BR> IoInitializeRemoveLock(
&p_ext->remove_lock, 'bilc', 15, 1000
);<BR>+ IoInitializeRemoveLock( &p_ext->stop_lock, 'dtci', 0, 1000
);<BR> <BR> /* Initialize the PnP states.
*/<BR> p_ext->pnp_state = NotStarted;<BR>@@ -428,15 +430,6
@@<BR> if( NT_SUCCESS( status )
)<BR> cl_set_pnp_state( p_ext, Started
);<BR> <BR>- /*<BR>- * If we get the start request when we're
already started, don't <BR>- * re-initialize the stop lock.<BR>-
*/<BR>- if( p_ext->last_pnp_state != Started )
{<BR>- CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl,
("IoInitializeRemoveLock: stop_lock %p[\n",
&p_ext->stop_lock));<BR>- IoInitializeRemoveLock(
&p_ext->stop_lock, 'dtci', 0, 1000
);<BR>- }<BR>-<BR> CL_EXIT( CL_DBG_PNP, p_ext->dbg_lvl
);<BR> return status;<BR> }<BR>@@ -553,33 +546,6
@@<BR> CL_TRACE_EXIT( CL_DBG_PNP,
p_ext->dbg_lvl,<BR> ("IRP_MN_CANCEL_STOP_DEVICE
received in invalid state.\n") );<BR> return
status;<BR>- }<BR>-<BR>- if( p_ext->last_pnp_state == Started
)<BR>- {<BR>- /*<BR>- * Re-initialize the stop
lock before rolling back the PnP<BR>- * state so that there's no
contention while it's uninitialized.<BR>-
*/<BR>- CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl,
("IoInitializeRemoveLock: stop_lock %p[\n",
&p_ext->stop_lock));<BR>- IoInitializeRemoveLock(
&p_ext->stop_lock, 'dtci', 0, 1000 );<BR>-#if
0 <BR>- // leo: it seems like a bug, because it can
never get released<BR>- {<BR>- /* <BR>- *
Acquire the stop lock to allow releasing and waiting when
stopping.<BR>-
*/<BR>- NTSTATUS status1;<BR>- CL_TRACE(
CL_DBG_PNP, p_ext->dbg_lvl, ("IoAcquireRemoveLock: stop_lock %p[\n",
&p_ext->stop_lock));<BR>- status1 =
IoAcquireRemoveLock( &p_ext->stop_lock, NULL
);<BR>- if( !NT_SUCCESS( status1 )
)<BR>- {<BR>- CL_TRACE( CL_DBG_ERROR,
p_ext->dbg_lvl, <BR>- ("IoAcquireRemoveLock
returned %08x. Continue anyway ...\n", status)
);<BR>- }<BR>- CL_TRACE( CL_DBG_PNP,
p_ext->dbg_lvl, ("IoAcquireRemoveLock: stop_lock
]\n"));<BR>- }<BR>-#endif <BR> }<BR> <BR> /*
Return to the previous PnP state. */<BR></DIV></FONT></SPAN>
<DIV dir=ltr align=left><SPAN class=082104615-02092009><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV><BR>
<DIV class=OutlookMessageHeader lang=en-us dir=ltr align=left>
<HR tabIndex=-1>
<FONT face=Tahoma size=2><B>From:</B> Leonid Keller
[mailto:leonid@mellanox.co.il] <BR><B>Sent:</B> Monday, August 17, 2009 5:29
AM<BR><B>To:</B> Fab Tillier; Smith, Stan; Hefty, Sean<BR><B>Cc:</B>
ofw_list<BR><B>Subject:</B> FW: [ofw][patch][IBBUS] crash on re-initialization
of remove lock<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>I
think this patch should go also into the branch.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>I'll
be on vacation 08/19-26, so i'll appreciate a lot if you could look it through
and opine.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>Also
if someone could check it, running WHQL tests ...</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN
class=757405711-17082009></SPAN></FONT> </DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN
class=757405711-17082009>There are two Remove Locks in IBBUS code. They are
called 'remove_lock' and 'stop_lock'.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>The
'stop_lock' get initialized twice, on device' start and
stop.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>The
'remove_lock' is initialized only once on the start up.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>In
my patch I did the same with 'stop_lock'.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=757405711-17082009>But
honestly, I don't quite understand why the author of the code didn't
do this in the first place ...</SPAN></FONT></DIV><BR>
<DIV class=OutlookMessageHeader lang=en-us dir=ltr align=left>
<HR tabIndex=-1>
<FONT face=Tahoma size=2><B>From:</B> ofw-bounces@lists.openfabrics.org
[mailto:ofw-bounces@lists.openfabrics.org] <B>On Behalf Of </B>Leonid
Keller<BR><B>Sent:</B> Sunday, August 16, 2009 7:46 PM<BR><B>To:</B>
ofw@lists.openfabrics.org<BR><B>Subject:</B> [ofw][patch][IBBUS] crash on
re-initialization of remove lock<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>I was reported a
crash upon running <FONT face="Times New Roman" size=3>“System Common
Scenario”</FONT> WHQL test with our stack.</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>The crash: C4
(0xd7), which means Driver Verifier revealed a re-initializing of Remove
Lock.</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>fffff880`01fa73a8
fffff800`0191b3dc : nt!KeBugCheckEx<BR>fffff880`01fa73b0 fffff800`0192d2e6 :
nt!NtShutdownSystem+0x7f0c<BR>fffff880`01fa73f0 fffff880`02a06974 :
nt!NtShutdownSystem+0x19e16<BR><STRONG>fffff880`01fa7450 fffff800`01937c16 :
ibbus!cl_pnp+0x1d8
[f:\ribel\mlnx_winof_2.0.5\4335\branches\mlnx_winof_2-0\core\complib\kernel\cl_pnp_po.c
@ 211]<BR></STRONG>fffff880`01fa74b0 fffff800`0193a52a :
nt!NtShutdownSystem+0x24746<BR>fffff880`01fa7510 fffff800`01937c16 :
nt!NtShutdownSystem+0x2705a<BR></DIV>
<DIV></FONT></SPAN><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>It happens on
win2k8 R2.</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>One can see that
IBAL PnP code, written in 2002 (?) contains in fact re-initialization of
stop Remove Lock.</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>Maybe sometimes it
was possible and was not checked by Driver Verifier or we just haven't come
accross it...</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>Here is a patch
that fixes the problem by eliminating re-initialization of the stop
lock.</FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial size=2>Index:
core/bus/kernel/bus_pnp.c<BR>===================================================================<BR>---
core/bus/kernel/bus_pnp.c (revision 4659)<BR>+++
core/bus/kernel/bus_pnp.c (working copy)<BR>@@ -216,7 +216,6
@@<BR> RtlZeroMemory(p_ext, sizeof
*p_ext);<BR> cl_init_pnp_po_ext( g_ControlDeviceObject,
NULL, <BR> NULL,
bus_globals.dbg_lvl, NULL, NULL
);<BR>- IoInitializeRemoveLock(
&p_ext->cl_ext.stop_lock, 'dtci', 0, 1000
);<BR> <BR> /* enable user-mode access to IB stack
*/<BR> BUS_PRINT( BUS_DBG_PNP, ("Remove-n-reCreate
dos_name symlink\n") );<BR>Index:
core/complib/kernel/cl_pnp_po.c<BR>===================================================================<BR>---
core/complib/kernel/cl_pnp_po.c (revision 4659)<BR>+++
core/complib/kernel/cl_pnp_po.c (working copy)<BR>@@ -137,7 +137,8
@@<BR> /* Store the pointer to our own device.
*/<BR> p_ext->p_self_do =
p_dev_obj;<BR> IoInitializeRemoveLock( &p_ext->remove_lock,
'bilc', 15, 1000 );<BR>-<BR>+ IoInitializeRemoveLock(
&p_ext->stop_lock, 'dtci', 0, 1000 );<BR>+ <BR> /*
Initialize the PnP states. */<BR> p_ext->pnp_state =
NotStarted;<BR> p_ext->last_pnp_state = NotStarted;<BR>@@
-428,15 +429,6 @@<BR> if( NT_SUCCESS( status )
)<BR> cl_set_pnp_state( p_ext, Started
);<BR> <BR>- /*<BR>- * If we get the start request when we're
already started, don't <BR>- * re-initialize the stop lock.<BR>-
*/<BR>- if( p_ext->last_pnp_state != Started )
{<BR>- CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl,
("IoInitializeRemoveLock: stop_lock %p[\n",
&p_ext->stop_lock));<BR>- IoInitializeRemoveLock(
&p_ext->stop_lock, 'dtci', 0, 1000
);<BR>- }<BR>-<BR> CL_EXIT( CL_DBG_PNP, p_ext->dbg_lvl
);<BR> return status;<BR> }<BR>@@ -557,12 +549,6
@@<BR> <BR> if( p_ext->last_pnp_state == Started
)<BR> {<BR>- /*<BR>- * Re-initialize the
stop lock before rolling back the PnP<BR>- * state so that there's
no contention while it's uninitialized.<BR>-
*/<BR>- CL_TRACE( CL_DBG_PNP, p_ext->dbg_lvl,
("IoInitializeRemoveLock: stop_lock %p[\n",
&p_ext->stop_lock));<BR>- IoInitializeRemoveLock(
&p_ext->stop_lock, 'dtci', 0, 1000 );<BR> #if
0 <BR> // leo: it seems like a bug, because it can
never get released<BR> {<BR></FONT></SPAN></DIV>
<DIV><SPAN class=232323116-16082009></SPAN> </DIV>
<DIV><SPAN class=232323116-16082009><FONT face=Arial
size=2></FONT></SPAN> </DIV></BLOCKQUOTE></BODY></HTML>