From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/5] drm/sysfs: protect sysfs removal code against being run twice. Date: Tue, 21 Feb 2012 09:39:17 +0100 Message-ID: <20120221083917.GB4065@phenom.ffwll.local> References: <1329754429-5491-1-git-send-email-airlied@gmail.com> <1329754429-5491-3-git-send-email-airlied@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 5F65D9ED7B for ; Tue, 21 Feb 2012 00:39:01 -0800 (PST) Received: by wera10 with SMTP id a10so4468351wer.36 for ; Tue, 21 Feb 2012 00:39:00 -0800 (PST) Content-Disposition: inline In-Reply-To: <1329754429-5491-3-git-send-email-airlied@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Dave Airlie Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Mon, Feb 20, 2012 at 04:13:46PM +0000, Dave Airlie wrote: > From: Dave Airlie > > 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 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48