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

Uri Habusha urih at mellanox.co.il
Sun Dec 5 06:25:33 PST 2010


SB

-----Original Message-----
From: Hefty, Sean [mailto:sean.hefty at intel.com] 
Sent: Thursday, December 02, 2010 11:54 PM
To: Uri Habusha; ofw at lists.openfabrics.org; Smith, Stan; Fab Tillier
Subject: RE: Add Assert to debug BS while deleting WDF device

> +	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.
> 
> [Uri Habusha] It will not resolve the problem. Let's assume 2 instances of the function is called. The
> first one is set the ctrldev under the lock than come second process and set the ctrldev. Now you try
> to delete the object twice. If you really think that multiple call of the function you should take the
> ctrldev and then set the ControlDevice to NULL under the lock.

Only the 'last' power exit call -- the one that removes the last device from DevList -- will attempt to delete the device.  This is why the check is 'if (destroy && ctrldev)'.  destroy is set to true only if DevList is empty.  The '&& ctrldev' should handle the case where power entry failed to create ControlDevice successfully.

[Uri Habusha] So if destroy is only set once under lock, why you need to save ControlDevice variable. You can use it directly. 
[Uri Habusha] 

We can't delete the ctrldev until all active users have been kicked off it, and there may be active read/write operations against the ctrldev when power exit is called.  So, we can't hold 'Lock' throughout power exit.  We have to wait until all threads return to userspace, and they may need to acquire Lock to do so.

To complicate matters, we could receive a power entry call for a different device at the same time we remove the last device.  The code must handle this as well.  We can't really assert anything about ControlDevice in power entry or exit.
[Uri Habusha] I don't understand why you can't ASSERT. If you try to Create device the ControlDevice should be NULL otherwise you have a leak.

- Sean



More information about the ofw mailing list