[ofw] Add Assert to debug BS while deleting WDF device

Hefty, Sean sean.hefty at intel.com
Thu Dec 2 08:21:48 PST 2010


copying your patch inline for comments.  See '>>' below:

Index: winmad/kernel/wm_driver.c
===================================================================
--- winmad/kernel/wm_driver.c	(revision 3009)
+++ winmad/kernel/wm_driver.c	(working copy)
@@ -41,7 +41,7 @@
 WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WM_IB_DEVICE, WmIbDeviceGetContext)
 WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WM_PROVIDER, WmProviderGetContext)
 
-WDFDEVICE				ControlDevice;
+WDFDEVICE				ControlDevice = NULL;


>> Globals are initialized to 0 by default.  This assignment isn't needed.



 static LIST_ENTRY		DevList;
 static LIST_ENTRY		ProvList;
 static KGUARDED_MUTEX	Lock;
@@ -195,11 +195,13 @@
 	WdfDeviceInitSetFileObjectConfig(pinit, &fileconfig, &attr);
 
 	WDF_OBJECT_ATTRIBUTES_INIT(&attr);
+	ASSERT(ControlDevice == NULL);
 	status = WdfDeviceCreate(&pinit, &attr, &ControlDevice);
 	if (!NT_SUCCESS(status)) {
 		goto err1;
 	}
 
+	ASSERT(ControlDevice != NULL);
 	status = WdfDeviceCreateSymbolicLink(ControlDevice, &symlink);
 	if (!NT_SUCCESS(status)) {
 		goto err2;
@@ -220,6 +222,7 @@
 
 err2:
 	WdfObjectDelete(ControlDevice);
+	ControlDevice = NULL;
 	return;
 err1:
 	WdfDeviceInitFree(pinit);
@@ -299,7 +302,8 @@
 
 	status = WdfFdoQueryForInterface(Device, &GUID_RDMA_INTERFACE_VERBS,
 									 (PINTERFACE) &dev->VerbsInterface,
-									 sizeof(dev->VerbsInterface), VerbsVersion(2, 0),
+ 									 sizeof(dev->VerbsInterface), 
+ 									 VerbsVersion(VERBS_MAJOR_VER, VERBS_MINOR_VER), 
 									 NULL);
 	if (!NT_SUCCESS(status)) {
 		return status;
@@ -340,14 +344,12 @@
 	WM_REGISTRATION		*reg;
 	LIST_ENTRY			*entry;
 	BOOLEAN				destroy;
-	WDFDEVICE			ctrldev;
 
 	pdev = WmIbDeviceGetContext(Device);
 
 	KeAcquireGuardedMutex(&Lock);
 	RemoveEntryList(&pdev->Entry);
 	destroy = IsListEmpty(&DevList);
-	ctrldev = ControlDevice;
 
 	for (entry = ProvList.Flink; entry != &ProvList; entry = entry->Flink) {
 		prov = CONTAINING_RECORD(entry, WM_PROVIDER, Entry);
@@ -364,8 +366,9 @@
 		ExFreePoolWithTag(pdev->pPortArray, 'pimw');
 	}
 
-	if (destroy) {
-		WdfObjectDelete(ctrldev);
+	if (destroy && (ControlDevice != NULL)) {
+		WdfObjectDelete(ControlDevice);
+		ControlDevice = NULL;

>> This change breaks the locking that was there.  We set ctrldev under lock and use that rather than ControlDevice directly outside of the lock.  If there are multiple HCAs in a system, then we could be processing power enter on one, while getting power exit for the other.  The locking needs to account for this, and this is why the code is structured as it is.  Because of this, I don't know that the assertions that you added will be true.

>> If all device power callbacks are serialized, we can simplify the code, but I couldn't find any documentation that clarifies whether this is the case.  The same issue exists in winverbs with your patches.  Can you please try running only with the changes that I submitted?

- Sean
 



More information about the ofw mailing list