> I got a BS while disabling the IPoIB driver, the reason for that is trying
> to delete WDF device that is invalid. I guess that the object already
> deleted but I'm not sure.
We need to understand what exactly is happening. The winmad and winverbs
drivers to not interact with ipoib, so I don't understand the relation.
We need to be sure that any patch is fixing the underlying issue. What
WdfOjbectDelete() call caused the crash? Is it coming from one of the error
handling sections(e.g. WmcreateControlDevice)? Setting ControlDevice = NULL
without any other checks should just result in calling the function with a NULL
pointer, so I don't understand the fix here.
For both drivers, the number of devices on the DevList indicates whether we
need to create or destroy the control device.
> Is WmPowerD0Exit and WmPowerD0Entry functions are thread safe? In other
> words is only one instance of the function can be run in same time?
I assume they would have to be -- for a single device. Otherwise, I don't know
what a driver could do with them. It wouldn't know what power state the device
was left in.
If there are multiple devices in the system, I don't know that the power state
callbacks are synchronized between them. This is the reason for the
synchronization that you see in the code. If PowerD0Entry/PowerD0Exit are
serialized, then the code can be simplified. We wouldn't need the
create/destroy variables at all in those cases.
> In order to catch the problem I added some ASSERT in the code and set the
> pointer to NULL after deleting the object.
Clearing the pointer is fine if needed, but IMO the asserts just add noise to
the code. The code isn't doing anything complex enough to justify asserts
everywhere.
> Index: winmad/kernel/wm_driver.c
>
> ===================================================================
>
> --- winmad/kernel/wm_driver.c (revision 6842)
>
> +++ 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;
global variables are initialized to NULL by default.
> err2:
>
> WdfObjectDelete(ControlDevice);
>
> + ControlDevice = NULL;
>
> return;
>
> err1:
>
> WdfDeviceInitFree(pinit);
>
> @@ -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);
>
> @@ -365,7 +367,8 @@
>
> }
>
>
>
> if (destroy) {
>
> - WdfObjectDelete(ctrldev);
>
> + ASSERT(ControlDevice != NULL);
>
> + WdfObjectDelete(ControlDevice);
This accesses ControlDevice outside of the lock. This assumes that no other
thread will call into the PowerD0* handlers. If this assumption is correct, we
can simplify the code even more.
- Sean
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
