Re: Device core removal ordering brokenness

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Sun, 2009-05-10 at 10:58 -0400, Alan Stern wrote:

> Before the patch, the ordering was like this:
> 
> device_add: 	ADD  dpm_sysfs_add()  ->probe
> device_del:	dpm_sysfs_remove()  ->remove  DEL

Right.

> Now the ordering is like this:
> 
> device_add:	dpm_sysfs_add()  ADD  ->probe
> device_del:	DEL  dpm_sysfs_remove()  ->remove
> 
> Okay, yes, it's not symmetrical.  But the point of the patch was to put
> the DEL before the dpm_sysfs_remove(), and in any case the code wasn't
> symmetrical even before the patch.

How so ? It does definitely look symetrical above :-) That's not a big
deal per-se though, it's just that I want to be able to tear down data
structures in DEL that may be indirectly used by the driver (DMA mapping
related or even MMIO related internal arch stuff).

>   I gather that you'd prefer to see
> 
> device_del:	->remove  DEL  dpm_sysfs_remove()

I don't actually care that much about where drm_sysfs_remove() is vs.
DEL, but you seem to want to adjust the sysfs files in ADD and DEL, so
that would make sense.

> Offhand I can't think of any reason not to do this.  Maybe someone else 
> can; this code has a lot of undocumented constraints.  (Hmm, what 
> happens if a system suspend occurs after the device has been 
> unregistered from its bus but before it has been taken off the dpm 
> list?  It's probably okay but worth checking...)
> 
> If you'd like to submit a patch moving the "if (dev->bus)...", 
> device_pm_remove(), and dpm_sysfs_remove() stuff after the call to 
> bus_remove_device(), go ahead.

I first want to "probe" you guys in case there's some nasty skeleton
waiting around the corner, but yeah I'll probably do that. One other
option is to split DEL in two.

Ben