On Mon, Feb 20, 2012 at 04:13:46PM +0000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> a step towards correct hot unplug for USB devices, we need to
> remove the userspace facing bits at the unplug time for correct
> udev operation.
>
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
The lack of any WARN_ON(mutex_is_locked) here and the funny comment about
the sysfs vs. hotunplug deadlock in the next patch makes me a bit uneasy.
I think the only issue race we need to care about here is a hotremove vs.
module unload race. Everything else should stay around for long enough
(although this might not quite be true with the current code ...). Simply
add a dev->hotremove_mutex and grabbing it in these two functions should
fix this.
It's a bit a funky locking scheme, but imo it should keep on working even
when properly fix up all our setup/teardown issues:
- setup just needs to properly order ctors, i.e. publish the userspace
interfaces only after everything is properly set up - we don't need any
locking to achieve that (well, currently we still rely massively on the
global_mutex dutcttape).
- concurrent removal of the userspace interfaces (like here) would be
protected by the new hotremove_mutex lock.
- protection against the underlying device disappearing would be handled
by dev->unplugged (I have some more comments about that)
- and the actual drm_device teardown controlled by reference-counting.
Currently we get away with abusing the fd open count, but I think sooner
or later we want a real reference count (e.g. when exporting buffers
with dma_buf we can't just reap them when the last user disappears).
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_sysfs.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 62c3675..5a7bd51 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -454,6 +454,8 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
> {
> int i;
>
> + if (!connector->kdev.parent)
> + return;
> DRM_DEBUG("removing \"%s\" from sysfs\n",
> drm_get_connector_name(connector));
>
> @@ -461,6 +463,7 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
> device_remove_file(&connector->kdev, &connector_attrs[i]);
> sysfs_remove_bin_file(&connector->kdev.kobj, &edid_attr);
> device_unregister(&connector->kdev);
> + connector->kdev.parent = NULL;
> }
> EXPORT_SYMBOL(drm_sysfs_connector_remove);
>
> @@ -533,7 +536,9 @@ err_out:
> */
> void drm_sysfs_device_remove(struct drm_minor *minor)
> {
> - device_unregister(&minor->kdev);
> + if (minor->kdev.parent)
> + device_unregister(&minor->kdev);
> + minor->kdev.parent = NULL;
> }
>
>
> --
> 1.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Mail: daniel@xxxxxxxx
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
