<!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.6000.16809" name=GENERATOR></HEAD>
<BODY dir=ltr>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2>Hello,</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>
Your ControlDeviceObject (CDO) implementation is very clean, nice
job.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>I
suspect the vstat failure I was seeing may have been due to the fact I was
creating the CDO in the add_device() routine where you, and I
agree, create the CDO in the DriverEntry() routine.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>Bottom
line - I do not see the previous vstat failure with your CDO
implementation.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2>Two small points:</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>1)
When disabling an HCA via the device manager, the device manager view fails to
refresh and show the HCA as disabled.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>One
can force the refresh with a scan for hardware changes.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2> 2) When all HCA devices have been 'disabled', running vstat.exe
crashes the system (Svr2003, (MT23108 InfiniHost) * 2).</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>The
bus_drv_unload() routine is not called for the case where all HCA devices have
been disabled. Consequently the symbolic link to the CDO is still valid thus
allowing vstat ioctl() access to IBAL layers with no HCAs
and eventually a system crash.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2>A few observations on managing the DOS name symbolic
link.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>1)
There are numerous instances and initializations of dos_name throughout
ibbus code.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2> One could make 'dos_name' a global to reduce code
with a single init and cleanup dos_name usage.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>2)
make the ControlDeviceObject 'dev_name' a global (persistent) as it's needed
to create the dos_name symbolic link upon 1st HCA enable operation in
bus_add_device() and needed in bus_drv_unload() to free UNICODE string
memory.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>3)
Initialize the unicode string globals dos_name and dev_name once in
DriverEntry()</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>4) In bus_pnp.c:bus_add_device() for the case
of the 1st HCA being added (ic==1), create the dos_name symbolic link; see #5 as
the CDO is already present if enabling from a disabled
state.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>5) The
dos_name symbolic link should be destroyed in bus_pnp.c:fdo_release_resources()
when the last HCA is disabled as this will prevent vstat from crashing the
system; the CDO remains valid.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff size=2>6)
bus_drv_unload() should free Unicode string memory for globals g_CDO_dos_name
& g_CDO_dev_name</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2>Stan.</FONT></SPAN></DIV>
<DIV><SPAN class=378270218-14042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=378270218-14042009><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> Saturday, April 11, 2009 2:41
PM<BR><B>To:</B> Smith, Stan<BR><B>Cc:</B>
ofw@lists.openfabrics.org<BR><B>Subject:</B> RE: [ofw][patch][IBBUS] Fix for a
bug from Bugzilla 1367<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=075342420-11042009>This
solution is based on the assumption, that applications work with IBAL and not
with concrete HCAs.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=075342420-11042009>The
Control Device has to convey an IOCTL to IBAL and it will find the way
to pass it down to the appropriate device. </SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=075342420-11042009>I
tried VSTAT with one and two cards, it worked
correctly.</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN class=075342420-11042009>Could
you bring an example of IOCTL, that breaks this assumption ?</SPAN></FONT></DIV>
<DIV><FONT face=Arial color=#0000ff size=2><SPAN
class=075342420-11042009></SPAN></FONT> </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> Smith, Stan
[mailto:stan.smith@intel.com] <BR><B>Sent:</B> Tuesday, April 07, 2009 7:19
PM<BR><B>To:</B> Leonid Keller<BR><B>Cc:</B>
ofw@lists.openfabrics.org<BR><B>Subject:</B> RE: [ofw][patch][IBBUS] Fix for a
bug from Bugzilla 1367<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>Hi
Leo,</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2> This patch does not apply, per chance could you send the patched
files: bus_driver.c & bus_pnp.c so I can continue
testing?</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>I'm
also concerned about tracking of a 'valid' PDO in ioctl
handling.</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>The
CDO gets</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial
size=2>+ cl_init_pnp_po_ext( g_ControlDeviceObject,
NULL, <BR>+ NULL, bus_globals.dbg_lvl, NULL, NULL
);</FONT><BR></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>How
can the follow bus_driver.c bus_ioctl.c code function without a valid
PDO?</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>My
experiments showed vstat quit working unless the CDO 'always' contained a
valid p_next_do?</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2>Which exposed the issue of when removing and HCA, one needed to make
sure the HCA/PDO being removed was not the one used by the CDO. If so, then
the CDO.p_next_do needed to be refreshed with a 'valid'
PDO.</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff size=2>If
the HCA being removed is the 'last' HCA, then vstat should indeed
fail.</FONT></DIV></SPAN>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2> /* Only pass down if not handled and not PDO device.
*/<BR> if( status == STATUS_INVALID_DEVICE_REQUEST &&
p_ext->cl_ext.p_next_do
)<BR> {<BR> IoSkipCurrentIrpStackLocation( p_irp
);<BR> status = IoCallDriver( p_ext->cl_ext.p_next_do, p_irp
);<BR> }</FONT></SPAN></DIV>
<DIV><FONT face=Arial color=#0000ff size=2></FONT> </DIV>
<DIV><FONT face=Arial color=#0000ff size=2></FONT> </DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2>thanks,</FONT></SPAN></DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2></FONT></SPAN> </DIV>
<DIV><SPAN class=329220616-07042009><FONT face=Arial color=#0000ff
size=2>Stan.</FONT></SPAN></DIV>
<DIV><BR></DIV>
<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> Sunday, April 05, 2009 6:47
AM<BR><B>To:</B> Smith, Stan<BR><B>Cc:</B>
ofw@lists.openfabrics.org<BR><B>Subject:</B> [ofw][patch][IBBUS] Fix for a bug
from Bugzilla 1367<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><FONT face=Arial size=2>Bugzilla 1367: If N cards are installed on a
machine, a disabling of the first one will<BR>prevent the work of all
applications with the rest cards.</FONT></DIV>
<DIV><FONT face=Arial size=2></FONT> </DIV>
<DIV><FONT face=Arial size=2>Explanation:<BR> IBBUS driver creates
only first instance of IBAL with a name. All the rest<BR>are unnamed. After
removing the first card, the named instance is gone and<BR>applications, which
are binding to IBAL by name, can't find it and can't work.</FONT></DIV>
<DIV><FONT face=Arial size=2></FONT> </DIV>
<DIV><FONT face=Arial size=2>Solution: The fix creates IBBUS Control Device of
"<A href="file://\\DosDevices\\Global\\ibal">\\DosDevices\\Global\\ibal</A>"
name from IBBUS Driver Entry.<BR>All other devices are created
unnamed.</FONT></DIV>
<DIV><FONT face=Arial size=2></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN
class=464114013-05042009></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2></FONT> </DIV>
<DIV><FONT face=Arial size=2>Index:
bus/kernel/bus_driver.c<BR>===================================================================<BR>---
bus/kernel/bus_driver.c (revision 2076)<BR>+++
bus/kernel/bus_driver.c (working copy)<BR>@@ -60,6 +60,7
@@<BR> <BR> <BR> char node_desc[IB_NODE_DESCRIPTION_SIZE];<BR>+PDEVICE_OBJECT g_ControlDeviceObject=NULL;<BR> <BR> bus_globals_t bus_globals
= {<BR> BUS_DBG_ERROR,<BR>@@ -981,6 +982,11
@@<BR> <BR> RtlInitUnicodeString( &dos_name, L"<A
href="file://\\DosDevices\\Global\\ibal">\\DosDevices\\Global\\ibal</A>"
);<BR> IoDeleteSymbolicLink( &dos_name );<BR>+ if
(g_ControlDeviceObject)<BR>+ {<BR>+ IoDeleteDevice(g_ControlDeviceObject);<BR>+ g_ControlDeviceObject
= NULL; <BR>+ }<BR> cur_conf =
bus_globals.p_pkey_conf;<BR> while(cur_conf)<BR> {<BR>@@
-1055,7 +1061,46 @@<BR> g_bfi_InstanceCount =
0;<BR> memset( __out_bcount(sizeof(g_bus_filters))
(void*)g_bus_filters, 0,<BR> sizeof(g_bus_filters)
);<BR>+ <BR>+ {<BR>+ UNICODE_STRING dev_name,
dos_name;<BR>+ bus_fdo_ext_t *p_ext=NULL;<BR> <BR>+ RtlInitUnicodeString(
&dev_name, AL_DEVICE_NAME );<BR>+ RtlInitUnicodeString(
&dos_name, L"<A
href="file://\\DosDevices\\Global\\ibal">\\DosDevices\\Global\\ibal</A>"
);<BR>+<BR>+ status = IoCreateDevice( p_driver_obj,
sizeof(bus_fdo_ext_t),<BR>+ &dev_name,
FILE_DEVICE_BUS_EXTENDER,<BR>+ FILE_DEVICE_SECURE_OPEN,
FALSE, &g_ControlDeviceObject );<BR>+ if( !NT_SUCCESS(status)
)<BR>+ {<BR>+ g_ControlDeviceObject =
NULL;<BR>+ BUS_PRINT( BUS_DBG_ERROR,
<BR>+ ("Failed to create ControlDeviceObject, status
%x.\n",status) );<BR>+ }<BR>+ else
{<BR>+ IoDeleteSymbolicLink( &dos_name
);<BR>+ status = IoCreateSymbolicLink( &dos_name,
&dev_name );<BR>+ if( !NT_SUCCESS(status)
)<BR>+ {<BR>+ IoDeleteDevice(
g_ControlDeviceObject );<BR>+ g_ControlDeviceObject =
NULL;<BR>+ BUS_PRINT(
BUS_DBG_ERROR,<BR>+ ("Failed to create symlink
for dos name.\n") );<BR>+ }<BR>+ else
{<BR>+ BUS_PRINT(
BUS_DBG_ERROR,<BR>+ ("SUCCEEDED to create symlink
for ControlDeviceObject.\n")
);<BR>+ <BR>+ p_ext =
g_ControlDeviceObject->DeviceExtension;<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>+ }<BR>+ }<BR>+ <BR> BUS_EXIT(
BUS_DBG_DRV );<BR> return STATUS_SUCCESS;<BR> }<BR>Index:
bus/kernel/bus_pnp.c<BR>===================================================================<BR>---
bus/kernel/bus_pnp.c (revision 2077)<BR>+++
bus/kernel/bus_pnp.c (working copy)<BR>@@ -60,6 +60,7
@@<BR> bus_filter_t g_bus_filters[MAX_BUS_FILTERS];<BR> <BR> extern
ib_al_handle_t gh_al; // NULL if AL needs init.<BR>+extern
PDEVICE_OBJECT g_ControlDeviceObject;<BR> <BR> <BR> static
NTSTATUS<BR>@@ -197,7 +198,7 @@<BR> /* Create the FDO device object
to attach to the stack. */<BR> <BR> /* if 1st Bus Filter
Instance, then create device names for user ioctl */<BR>- if ( ic == 1
)<BR>+ if ( ic == 1 &&
!g_ControlDeviceObject)<BR> {<BR> RtlInitUnicodeString(
&dev_name, AL_DEVICE_NAME );<BR> RtlInitUnicodeString(
&dos_name, L"<A
href="file://\\DosDevices\\Global\\ibal">\\DosDevices\\Global\\ibal</A>"
);<BR></FONT></DIV></BLOCKQUOTE></BODY></HTML>