[Openib-windows] [RFC] changing kernel complib to be static l ibrary
Leonid Keller
leonid at mellanox.co.il
Wed Jul 13 10:05:54 PDT 2005
i don't like the idea.
Windows' kernel consists of DLLs and they change it with service packs
anyway ...
And what about IBAL.sys - it is also used now by several components ?
Using a static library can bring us to kind of "DLL mess" problem, when each
driver works with its own version of COMPLIB and a bug, fixed in one
version, continues to be present in others.
May be we can use another solutions, say:
- add versioning to COMPLIB , so a new HCA driver will fail loading
upon unappropriate COMPLIB version;
- add checking of build_types of the components (also on startup);
- (the worst case) to copy all the files to system\drivers upon
driver update and require reboot! :(
-----Original Message-----
From: Fab Tillier [mailto:ftillier at silverstorm.com]
Sent: Tuesday, July 12, 2005 11:32 PM
To: openib-windows at openib.org
Subject: [Openib-windows] [RFC] changing kernel complib to be static
library
Folks,
Currently, the kernel level component library driver is a kernel DLL. This
places a few restrictions on the user:
1. All kernel drivers must be of the same build type as complib. Unmatched
drivers can put the system in a state where it crashes on boot, and safe
mode
doesn't always help. Recovering from this is really difficult, and it's
almost
easier to just re-install the system (yes, that's how bad this is).
2. Driver updates don't always properly track DLL dependencies, so updating
the
complib DLL might not take effect, resulting in odd behavior. Say someone
updates the HCA driver, which installs a new version of complib driver to
disk.
If there are any existing drivers loaded with a dependency on complib, the
complib driver won't be unloaded. When the HCA driver starts, it will end
up
using the existing complib driver, not the updated one. This could be
messy,
depending on the update to complib.
The fix for this is to make complib a static library, but the result is
larger
binary footprint (1% total, but up to 50% for some drivers for release
builds),
and likely a larger memory footprint due to things being duplicated. This
is
exacerbated in debug builds, where debug code is duplicated in each driver
(e.g.
the memory tracker).
Lastly, changing to a static library requires changing drivers to explicitly
initialize and cleanup global complib stuff (like the memory tracker and
object
manager).
Below is a patch that does this. Please let me know if this seems
worthwhile,
and if so, if there are any issues or improvements.
Thanks,
- Fab
Index: ulp/ipoib/kernel/SOURCES
===================================================================
--- ulp/ipoib/kernel/SOURCES (revision 36)
+++ ulp/ipoib/kernel/SOURCES (working copy)
@@ -11,7 +11,7 @@
INCLUDES=..;..\..\..\inc;..\..\..\inc\kernel;
C_DEFINES=$(C_DEFINES) -DNDIS_MINIPORT_DRIVER -DNDIS_WDM=1 \
- -DDEPRECATE_DDK_FUNCTIONS -DNDIS51_MINIPORT
+ -DDEPRECATE_DDK_FUNCTIONS -DNDIS51_MINIPORT -DNEED_CL_OBJ
TARGETLIBS= \
$(TARGETPATH)\*\complib.lib \
Index: ulp/ipoib/kernel/ipoib_adapter.c
===================================================================
--- ulp/ipoib/kernel/ipoib_adapter.c (revision 36)
+++ ulp/ipoib/kernel/ipoib_adapter.c (working copy)
@@ -129,9 +129,17 @@
IPOIB_ENTER( IPOIB_DBG_INIT );
+ if( !NT_SUCCESS( CL_INIT ) )
+ {
+ IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,
+ ("cl_init failed.\n") );
+ return IB_ERROR;
+ }
+
p_adapter = cl_zalloc( sizeof(ipoib_adapter_t) );
if( !p_adapter )
{
+ CL_DEINIT;
IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,
("Failed to allocate ipoib_adapter_t (%d bytes)",
sizeof(ipoib_adapter_t)) );
@@ -455,6 +463,8 @@
cl_free( p_adapter );
+ CL_DEINIT;
+
IPOIB_EXIT( IPOIB_DBG_INIT );
}
Index: ulp/ipoib/kernel/ipoib.rc
===================================================================
--- ulp/ipoib/kernel/ipoib.rc (revision 40)
+++ ulp/ipoib/kernel/ipoib.rc (working copy)
@@ -26,7 +26,7 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*
- * $Id$
+ * $Id: ipoib.rc 1.13 05/07/12 10:46:54-07:00
ftillier at orbuild.infiniconsys.com
$
*/
Index: ulp/ipoib/kernel/ipoib_driver.c
===================================================================
--- ulp/ipoib/kernel/ipoib_driver.c (revision 36)
+++ ulp/ipoib/kernel/ipoib_driver.c (working copy)
@@ -516,6 +516,7 @@
if( medium_index == medium_array_size ) /* Never found it */
{
+ CL_DEINIT;
IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR, ("No supported media.\n")
);
return NDIS_STATUS_UNSUPPORTED_MEDIA;
}
Index: ulp/srp/kernel/SOURCES
===================================================================
--- ulp/srp/kernel/SOURCES (revision 36)
+++ ulp/srp/kernel/SOURCES (working copy)
@@ -16,9 +16,10 @@
!if defined(DDK_TARGET_OS) && "$(DDK_TARGET_OS)"=="WinXP"
# storport.h in WinXP DDK already have "..._ALIASES" definition
-C_DEFINES=$(C_DEFINES) -DDEPRECATE_DDK_FUNCTIONS -DWinXP
+C_DEFINES=$(C_DEFINES) -DDEPRECATE_DDK_FUNCTIONS -DWinXP -DNEED_CL_OBJ
!else
-C_DEFINES=$(C_DEFINES) -DDEPRECATE_DDK_FUNCTIONS -DSTOR_USE_SCSI_ALIASES
+C_DEFINES=$(C_DEFINES) -DDEPRECATE_DDK_FUNCTIONS -DSTOR_USE_SCSI_ALIASES \
+ -DNEED_CL_OBJ
!endif
TARGETLIBS= \
Index: ulp/srp/kernel/srp_driver.c
===================================================================
--- ulp/srp/kernel/srp_driver.c (revision 36)
+++ ulp/srp/kernel/srp_driver.c (working copy)
@@ -204,12 +204,20 @@
IN DRIVER_OBJECT *p_drv_obj,
IN UNICODE_STRING *p_registry_path )
{
- ULONG status =
(ULONG)STATUS_INSUFFICIENT_RESOURCES;
+ ULONG status;
HW_INITIALIZATION_DATA hw_data;
cl_status_t cl_status;
SRP_ENTER( SRP_DBG_PNP );
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ SRP_TRACE_EXIT( SRP_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
gp_drv_obj = p_drv_obj;
cl_obj_construct( &g_drv_obj, SRP_OBJ_TYPE_DRV );
@@ -271,10 +279,16 @@
}
else
{
+ CL_DEINIT;
SRP_TRACE( SRP_DBG_ERROR,
("StorPortInitialize returned 0x%x.\n",
status)
);
}
}
+ else
+ {
+ CL_DEINIT;
+ status = (ULONG)STATUS_INSUFFICIENT_RESOURCES;
+ }
SRP_TRACE_EXIT( SRP_DBG_PNP, ("DriverEntry returning status of
0x%x.\n",
status) );
return status;
@@ -291,6 +305,8 @@
SRP_TRACE( SRP_DBG_VERBOSE, ("Driver Object ref_cnt = %d\n",
g_drv_obj.ref_cnt) );
cl_obj_destroy( &g_drv_obj );
+ CL_DEINIT;
+
/* Invoke the port driver's unload routine. */
SRP_TRACE( SRP_DBG_DEBUG, ("Invoking the port driver's unload
routine.\n") );
gpfn_unload( p_drv_obj );
Index: tests/alts/kernel/alts_driver.c
===================================================================
--- tests/alts/kernel/alts_driver.c (revision 36)
+++ tests/alts/kernel/alts_driver.c (working copy)
@@ -133,6 +133,7 @@
IN PDRIVER_OBJECT
p_driver_obj,
IN PUNICODE_STRING
p_registry_path )
{
+ NTSTATUS status;
#ifdef _DEBUG_
static boolean_t exit = FALSE;
#endif
@@ -150,6 +151,14 @@
}
#endif
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ ALTS_TRACE_EXIT( ALTS_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
p_driver_obj->MajorFunction[IRP_MJ_PNP] = cl_pnp;
p_driver_obj->MajorFunction[IRP_MJ_POWER] = cl_power;
// p_driver_obj->MajorFunction[IRP_MJ_DEVICE_CONTROL] = alts_ioctl;
@@ -170,6 +179,8 @@
UNUSED_PARAM( p_driver_obj );
+ CL_DEINIT;
+
ALTS_EXIT( ALTS_DBG_DEV );
}
Index: core/complib/kernel/SOURCES
===================================================================
--- core/complib/kernel/SOURCES (revision 36)
+++ core/complib/kernel/SOURCES (working copy)
@@ -1,6 +1,6 @@
TARGETNAME=complib
TARGETPATH=..\..\..\bin\kernel\obj$(BUILD_ALT_DIR)
-TARGETTYPE=EXPORT_DRIVER
+TARGETTYPE=DRIVER_LIBRARY
DLLDEF=cl_exports.def
Index: core/complib/kernel/cl_driver.c
===================================================================
--- core/complib/kernel/cl_driver.c (revision 36)
+++ core/complib/kernel/cl_driver.c (working copy)
@@ -33,43 +33,45 @@
#include "complib/comp_lib.h"
-NTSTATUS
-DriverEntry(
- IN DRIVER_OBJECT
*pDevObj,
- IN UNICODE_STRING
*RegistryPath )
-{
- UNUSED_PARAM( pDevObj );
- UNUSED_PARAM( RegistryPath );
- return STATUS_SUCCESS;
-}
+//
+//
+//NTSTATUS
+//DriverEntry(
+// IN DRIVER_OBJECT
*pDevObj,
+// IN UNICODE_STRING
*RegistryPath )
+//{
+// UNUSED_PARAM( pDevObj );
+// UNUSED_PARAM( RegistryPath );
+// return STATUS_SUCCESS;
+//}
+//
+//
+//NTSTATUS
+//DllInitialize(
+// IN UNICODE_STRING
*RegistryPath )
+//{
+// cl_status_t status;
+// UNUSED_PARAM( RegistryPath );
+//
+// __cl_mem_track( TRUE );
+//
+// status = cl_obj_mgr_create();
+//
+// return cl_to_ntstatus( status );
+//}
+//
+//
+//NTSTATUS
+//DllUnload(void)
+//{
+// cl_obj_mgr_destroy();
+//
+// __cl_mem_track( FALSE );
+//
+// return STATUS_SUCCESS;
+//}
-NTSTATUS
-DllInitialize(
- IN UNICODE_STRING
*RegistryPath )
-{
- cl_status_t status;
- UNUSED_PARAM( RegistryPath );
-
- __cl_mem_track( TRUE );
-
- status = cl_obj_mgr_create();
-
- return cl_to_ntstatus( status );
-}
-
-
-NTSTATUS
-DllUnload(void)
-{
- cl_obj_mgr_destroy();
-
- __cl_mem_track( FALSE );
-
- return STATUS_SUCCESS;
-}
-
-
CL_EXPORT NTSTATUS
cl_to_ntstatus(
IN cl_status_t status )
Index: core/complib/cl_memory.c
===================================================================
--- core/complib/cl_memory.c (revision 36)
+++ core/complib/cl_memory.c (working copy)
@@ -42,9 +42,11 @@
#include "cl_memtrack.h"
+#include <complib/cl_atomic.h>
cl_mem_tracker_t *gp_mem_tracker = NULL;
+atomic32_t g_mem_trk_ref = 0;
/*
@@ -73,7 +75,10 @@
cl_status_t status;
if( gp_mem_tracker )
+ {
+ cl_atomic_inc( &g_mem_trk_ref );
return;
+ }
/* Allocate the memory tracker object. */
gp_mem_tracker = (cl_mem_tracker_t*)
@@ -112,6 +117,9 @@
if( !gp_mem_tracker )
return;
+ if( cl_atomic_dec( &g_mem_trk_ref ) )
+ return;
+
if( cl_qmap_count( &gp_mem_tracker->alloc_map ) )
{
/* There are still items in the list. Print them out. */
Index: core/complib/cl_obj.c
===================================================================
--- core/complib/cl_obj.c (revision 36)
+++ core/complib/cl_obj.c (working copy)
@@ -43,6 +43,7 @@
/* The global object manager. */
cl_obj_mgr_t *gp_obj_mgr = NULL;
+atomic32_t g_cl_obj_ref = 0;
@@ -57,7 +58,10 @@
/* See if the object manager has already been created. */
if( gp_obj_mgr )
+ {
+ cl_atomic_inc( &g_cl_obj_ref );
return CL_SUCCESS;
+ }
/* Allocate the object manager. */
gp_obj_mgr = cl_zalloc( sizeof( cl_obj_mgr_t ) );
@@ -70,6 +74,8 @@
cl_async_proc_construct( &gp_obj_mgr->async_proc_mgr );
cl_qpool_construct( &gp_obj_mgr->rel_pool );
+ cl_atomic_inc( &g_cl_obj_ref );
+
/* Initialize the spinlock. */
status = cl_spinlock_init( &gp_obj_mgr->lock );
if( status != CL_SUCCESS )
@@ -110,6 +116,10 @@
if( !gp_obj_mgr )
return;
+ /* See if this is the last call. */
+ if( cl_atomic_dec( &g_cl_obj_ref ) )
+ return;
+
/* Verify that all object's have been destroyed. */
for( p_list_item = cl_qlist_head( &gp_obj_mgr->obj_list );
p_list_item != cl_qlist_end( &gp_obj_mgr->obj_list );
Index: core/bus/kernel/bus_driver.c
===================================================================
--- core/bus/kernel/bus_driver.c (revision 36)
+++ core/bus/kernel/bus_driver.c (working copy)
@@ -327,6 +327,8 @@
UNUSED_PARAM( p_driver_obj );
+ CL_DEINIT;
+
BUS_EXIT( BUS_DBG_DRV );
}
@@ -340,6 +342,14 @@
BUS_ENTER( BUS_DBG_DRV );
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ BUS_TRACE_EXIT( BUS_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
/* Store the driver object pointer in the global parameters. */
bus_globals.p_driver_obj = p_driver_obj;
@@ -347,6 +357,7 @@
status = __read_registry( p_registry_path );
if( !NT_SUCCESS(status) )
{
+ CL_DEINIT;
BUS_TRACE_EXIT( BUS_DBG_ERROR,
("__read_registry returned %08x.\n", status) );
return status;
Index: core/al/kernel/al_driver.c
===================================================================
--- core/al/kernel/al_driver.c (revision 36)
+++ core/al/kernel/al_driver.c (working copy)
@@ -239,10 +239,19 @@
AL_ENTER( AL_DBG_DEV );
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ AL_TRACE_EXIT( AL_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
/* Get the registry values. */
status = __read_registry( p_registry_path );
if( !NT_SUCCESS(status) )
{
+ CL_DEINIT;
AL_TRACE_EXIT( AL_DBG_ERROR,
("__read_registry returned %08x.\n", status) );
return status;
@@ -258,6 +267,8 @@
{
AL_ENTER( AL_DBG_DEV );
+ CL_DEINIT;
+
AL_EXIT( AL_DBG_DEV );
return STATUS_SUCCESS;
}
Index: core/iou/kernel/iou_driver.c
===================================================================
--- core/iou/kernel/iou_driver.c (revision 36)
+++ core/iou/kernel/iou_driver.c (working copy)
@@ -188,6 +188,8 @@
UNUSED_PARAM( p_driver_obj );
+ CL_DEINIT;
+
IOU_EXIT( IOU_DBG_DRV );
}
@@ -201,6 +203,14 @@
IOU_ENTER( IOU_DBG_DRV );
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ IOU_TRACE_EXIT( IOU_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
/* Store the driver object pointer in the global parameters. */
iou_globals.p_driver_obj = p_driver_obj;
@@ -208,6 +218,7 @@
status = __read_registry( p_registry_path );
if( !NT_SUCCESS(status) )
{
+ CL_DEINIT;
IOU_TRACE_EXIT( IOU_DBG_ERROR,
("__read_registry returned %08x.\n", status) );
return status;
@@ -220,6 +231,8 @@
p_driver_obj->DriverUnload = iou_unload;
p_driver_obj->DriverExtension->AddDevice = iou_add_device;
+ status = cl_to_ntstatus( cl_obj_mgr_create() );
+
IOU_EXIT( IOU_DBG_DRV );
- return STATUS_SUCCESS;
+ return status;
}
Index: inc/kernel/complib/cl_types_osd.h
===================================================================
--- inc/kernel/complib/cl_types_osd.h (revision 36)
+++ inc/kernel/complib/cl_types_osd.h (working copy)
@@ -46,6 +46,9 @@
#include <wdmwarn4.h>
#if defined( NDIS_MINIPORT_DRIVER )
#include <ndis.h>
+#if NDIS_WDM
+#define CL_NTDDK
+#endif /* NDIS_WDM */
#elif !defined( _MINIPORT_ )
#include <ntddk.h>
#define CL_NTDDK
@@ -93,11 +96,7 @@
#define UNUSED_PARAM UNREFERENCED_PARAMETER
-#if !defined(EXPORT_CL_SYMBOLS)
-#define CL_EXPORT DECLSPEC_IMPORT
-#else
-#define CL_EXPORT __declspec(dllexport)
-#endif
+#define CL_EXPORT
#if !defined( __cplusplus )
#define inline __inline
@@ -119,15 +118,33 @@
#define CL_CONST64( x ) x##ui64
-CL_INLINE NTSTATUS
+NTSTATUS
cl_to_ntstatus(
IN enum _cl_status status );
-CL_INLINE enum _cl_status
+enum _cl_status
cl_from_ntstatus(
IN NTSTATUS status );
+#ifdef MEM_TRACK_ON
+#ifdef NEED_CL_OBJ
+#define CL_INIT __cl_mem_track( TRUE ), cl_obj_mgr_create()
+#define CL_DEINIT cl_obj_mgr_destroy(), __cl_mem_track( FALSE )
+#else
+#endif /* NEED_CL_OBJ */
+#define CL_INIT __cl_mem_track( TRUE ), STATUS_SUCCESS
+#define CL_DEINIT __cl_mem_track( FALSE )
+#else /* MEM_TRACK_ON */
+#ifdef NEED_CL_OBJ
+#define CL_INIT cl_obj_mgr_create()
+#define CL_DEINIT cl_obj_mgr_destroy()
+#else
+#define CL_INIT STATUS_SUCCESS
+#define CL_DEINIT
+#endif /* NEED_CL_OBJ */
+#endif /* MEM_TRACK_ON */
+
#ifdef __cplusplus
} /* extern "C" */
#endif
Index: hw/mt23108/vapi/mlxsys/os_dep/win/tdriver/Md.c
===================================================================
--- hw/mt23108/vapi/mlxsys/os_dep/win/tdriver/Md.c (revision 36)
+++ hw/mt23108/vapi/mlxsys/os_dep/win/tdriver/Md.c (working copy)
@@ -255,6 +255,10 @@
/* no context yet */
g_pDrvContext = NULL;
+ l_Status = CL_INIT;
+ if( !NT_SUCCESS(l_Status) )
+ return l_Status;
+
/* create Control Device names */
if (!MdCreateDeviceNames(MD_CTL_DEVICE_NAME, &l_usNtDeviceName,
&l_usDosDeviceName))
{ /* failed - no resources */
@@ -410,6 +414,8 @@
MdKdPrint( DBGLVL_ALWAYS ,("(MdDeviceInit) Device failed to
initialize
\n"));
#pragma warning( pop )
+ CL_DEINIT;
+
/* Write to event log */
WriteEventLogEntry( pi_pDriverObject, MD_EVENT_LOG_LOAD_ERROR,
0, l_Status, 1, l_Status );
@@ -565,6 +571,9 @@
/* free the driver context */
MdExFreePool( l_pDrvContext );
+
+ CL_DEINIT;
+
g_pDrvContext = NULL;
MDASSERT( g_pDbgData->m_nExAllocCount == 0 );
Index: hw/mt23108/kernel/hca_driver.c
===================================================================
--- hw/mt23108/kernel/hca_driver.c (revision 36)
+++ hw/mt23108/kernel/hca_driver.c (working copy)
@@ -267,9 +267,18 @@
HCA_ENTER( HCA_DBG_DEV );
+ status = CL_INIT;
+ if( !NT_SUCCESS(status) )
+ {
+ HCA_TRACE_EXIT( HCA_DBG_ERROR,
+ ("cl_init returned %08X.\n", status) );
+ return status;
+ }
+
status = __read_registry( p_registry_path );
if( !NT_SUCCESS( status ) )
{
+ CL_DEINIT;
HCA_TRACE_EXIT( HCA_DBG_ERROR,
("__read_registry_path returned 0x%X.\n", status) );
return status;
@@ -279,6 +288,7 @@
cl_status = mlnx_hobs_init();
if( cl_status != CL_SUCCESS )
{
+ CL_DEINIT;
HCA_TRACE_EXIT( HCA_DBG_ERROR,
("mlnx_hobs_init returned %s.\n",
cl_status_text[cl_status]) );
return cl_to_ntstatus( cl_status );
@@ -353,6 +363,8 @@
UNUSED_PARAM( p_driver_obj );
+ CL_DEINIT;
+
HCA_EXIT( HCA_DBG_DEV );
}
_______________________________________________
openib-windows mailing list
openib-windows at openib.org
http://openib.org/mailman/listinfo/openib-windows
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20050713/cec47289/attachment.html>
More information about the ofw
mailing list