From: Sean Paul <seanpaul@chromium.org> To: Jeffy Chen <jeffy.chen@rock-chips.com> Cc: linux-kernel@vger.kernel.org, briannorris@chromium.org, dianders@chromium.org, tfiga@chromium.org, seanpaul@chromium.org, zyw@rock-chips.com, marcheu@chromium.org, mark.yao@rock-chips.com, hshi@chromium.org, Daniel Vetter <daniel.vetter@intel.com>, Jani Nikula <jani.nikula@linux.intel.com>, dri-devel@lists.freedesktop.org, Chris Wilson <chris@chris-wilson.co.uk>, David Airlie <airlied@linux.ie>, Tom Gundersen <teg@jklm.no>, Dave Airlie <airlied@redhat.com> Subject: Re: [PATCH v9] drm: Unplug drm device when unregistering it Date: Wed, 12 Apr 2017 11:03:21 -0400 [thread overview] Message-ID: <20170412150321.uqlbrkb55chajylq@art_vandelay> (raw) In-Reply-To: <1491987381-32533-2-git-send-email-jeffy.chen@rock-chips.com> On Wed, Apr 12, 2017 at 04:56:21PM +0800, Jeffy Chen wrote: > After unbinding drm, the user space may still owns the drm dev fd, and > may still be able to call drm ioctl. > > We're using an unplugged state to prevent something like that, so let's > reuse it here. > > Also drop drm_unplug_dev, because it would be unused after other changes. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > Changes in v9: > Move drm_device_set_plug_state into drm_drv.c . > > Changes in v8: > Fix hang when unregistering drm dev with open_count 0. > > Changes in v7: > Add drm_device_set_plug_state helper. > > Changes in v6: > Reuse unplug status. > > Changes in v5: > Fix wrong git account. > > Changes in v2: > Fix some commit messages. > Hi Jeffy, A couple bookkeeping notes: - Please end the commit message with your Signed-off-by tag - No need to send cover letters for one patch. All info should be in the commit message. Include superfluous information (such as tested on 4.4 kernel) after --- > --- > drivers/gpu/drm/drm_drv.c | 26 ++++++++++---------------- > drivers/gpu/drm/udl/udl_drv.c | 2 +- > include/drm/drmP.h | 6 ------ > include/drm/drm_drv.h | 1 - > 4 files changed, 11 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index b5c6bb4..e1da4d1 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -355,22 +355,6 @@ void drm_put_dev(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_put_dev); > > -void drm_unplug_dev(struct drm_device *dev) > -{ > - /* for a USB device */ > - drm_dev_unregister(dev); > - > - mutex_lock(&drm_global_mutex); > - > - drm_device_set_unplugged(dev); > - > - if (dev->open_count == 0) { > - drm_put_dev(dev); > - } > - mutex_unlock(&drm_global_mutex); > -} > -EXPORT_SYMBOL(drm_unplug_dev); > - > /* > * DRM internal mount > * We want to be able to allocate our own "struct address_space" to control > @@ -733,6 +717,13 @@ static void remove_compat_control_link(struct drm_device *dev) > kfree(name); > } > > +static inline void drm_device_set_plug_state(struct drm_device *dev, > + bool plugged) > +{ > + smp_wmb(); > + atomic_set(&dev->unplugged, !plugged); > +} > + > /** > * drm_dev_register - Register DRM device > * @dev: Device to register > @@ -787,6 +778,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > if (drm_core_check_feature(dev, DRIVER_MODESET)) > drm_modeset_register_all(dev); > > + drm_device_set_plug_state(dev, true); > + > ret = 0; > > DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", > @@ -826,6 +819,7 @@ void drm_dev_unregister(struct drm_device *dev) > drm_lastclose(dev); > > dev->registered = false; > + drm_device_set_plug_state(dev, false); > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > drm_modeset_unregister_all(dev); > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index cd8b017..5dbd916 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -108,7 +108,7 @@ static void udl_usb_disconnect(struct usb_interface *interface) > drm_kms_helper_poll_disable(dev); > udl_fbdev_unplug(dev); > udl_drop_usb(dev); > - drm_unplug_dev(dev); > + drm_dev_unregister(dev); I think you need drm_dev_unref here as well. Sean > } > > /* > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 3bfafcd..980a204 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -488,12 +488,6 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev, > return ((dev->driver->driver_features & feature) ? 1 : 0); > } > > -static inline void drm_device_set_unplugged(struct drm_device *dev) > -{ > - smp_wmb(); > - atomic_set(&dev->unplugged, 1); > -} > - > static inline int drm_device_is_unplugged(struct drm_device *dev) > { > int ret = atomic_read(&dev->unplugged); > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 0fefc3f..eb63078 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -544,7 +544,6 @@ void drm_dev_unregister(struct drm_device *dev); > void drm_dev_ref(struct drm_device *dev); > void drm_dev_unref(struct drm_device *dev); > void drm_put_dev(struct drm_device *dev); > -void drm_unplug_dev(struct drm_device *dev); > > int drm_dev_set_unique(struct drm_device *dev, const char *name); > > -- > 2.1.4 > -- Sean Paul, Software Engineer, Google / Chromium OS
WARNING: multiple messages have this Message-ID (diff)
From: Sean Paul <seanpaul@chromium.org> To: Jeffy Chen <jeffy.chen@rock-chips.com> Cc: dianders@chromium.org, Dave Airlie <airlied@redhat.com>, briannorris@chromium.org, linux-kernel@vger.kernel.org, tfiga@chromium.org, dri-devel@lists.freedesktop.org, Daniel Vetter <daniel.vetter@intel.com>, zyw@rock-chips.com, marcheu@chromium.org, hshi@chromium.org Subject: Re: [PATCH v9] drm: Unplug drm device when unregistering it Date: Wed, 12 Apr 2017 11:03:21 -0400 [thread overview] Message-ID: <20170412150321.uqlbrkb55chajylq@art_vandelay> (raw) In-Reply-To: <1491987381-32533-2-git-send-email-jeffy.chen@rock-chips.com> On Wed, Apr 12, 2017 at 04:56:21PM +0800, Jeffy Chen wrote: > After unbinding drm, the user space may still owns the drm dev fd, and > may still be able to call drm ioctl. > > We're using an unplugged state to prevent something like that, so let's > reuse it here. > > Also drop drm_unplug_dev, because it would be unused after other changes. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > Changes in v9: > Move drm_device_set_plug_state into drm_drv.c . > > Changes in v8: > Fix hang when unregistering drm dev with open_count 0. > > Changes in v7: > Add drm_device_set_plug_state helper. > > Changes in v6: > Reuse unplug status. > > Changes in v5: > Fix wrong git account. > > Changes in v2: > Fix some commit messages. > Hi Jeffy, A couple bookkeeping notes: - Please end the commit message with your Signed-off-by tag - No need to send cover letters for one patch. All info should be in the commit message. Include superfluous information (such as tested on 4.4 kernel) after --- > --- > drivers/gpu/drm/drm_drv.c | 26 ++++++++++---------------- > drivers/gpu/drm/udl/udl_drv.c | 2 +- > include/drm/drmP.h | 6 ------ > include/drm/drm_drv.h | 1 - > 4 files changed, 11 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index b5c6bb4..e1da4d1 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -355,22 +355,6 @@ void drm_put_dev(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_put_dev); > > -void drm_unplug_dev(struct drm_device *dev) > -{ > - /* for a USB device */ > - drm_dev_unregister(dev); > - > - mutex_lock(&drm_global_mutex); > - > - drm_device_set_unplugged(dev); > - > - if (dev->open_count == 0) { > - drm_put_dev(dev); > - } > - mutex_unlock(&drm_global_mutex); > -} > -EXPORT_SYMBOL(drm_unplug_dev); > - > /* > * DRM internal mount > * We want to be able to allocate our own "struct address_space" to control > @@ -733,6 +717,13 @@ static void remove_compat_control_link(struct drm_device *dev) > kfree(name); > } > > +static inline void drm_device_set_plug_state(struct drm_device *dev, > + bool plugged) > +{ > + smp_wmb(); > + atomic_set(&dev->unplugged, !plugged); > +} > + > /** > * drm_dev_register - Register DRM device > * @dev: Device to register > @@ -787,6 +778,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > if (drm_core_check_feature(dev, DRIVER_MODESET)) > drm_modeset_register_all(dev); > > + drm_device_set_plug_state(dev, true); > + > ret = 0; > > DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", > @@ -826,6 +819,7 @@ void drm_dev_unregister(struct drm_device *dev) > drm_lastclose(dev); > > dev->registered = false; > + drm_device_set_plug_state(dev, false); > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > drm_modeset_unregister_all(dev); > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index cd8b017..5dbd916 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -108,7 +108,7 @@ static void udl_usb_disconnect(struct usb_interface *interface) > drm_kms_helper_poll_disable(dev); > udl_fbdev_unplug(dev); > udl_drop_usb(dev); > - drm_unplug_dev(dev); > + drm_dev_unregister(dev); I think you need drm_dev_unref here as well. Sean > } > > /* > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 3bfafcd..980a204 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -488,12 +488,6 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev, > return ((dev->driver->driver_features & feature) ? 1 : 0); > } > > -static inline void drm_device_set_unplugged(struct drm_device *dev) > -{ > - smp_wmb(); > - atomic_set(&dev->unplugged, 1); > -} > - > static inline int drm_device_is_unplugged(struct drm_device *dev) > { > int ret = atomic_read(&dev->unplugged); > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 0fefc3f..eb63078 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -544,7 +544,6 @@ void drm_dev_unregister(struct drm_device *dev); > void drm_dev_ref(struct drm_device *dev); > void drm_dev_unref(struct drm_device *dev); > void drm_put_dev(struct drm_device *dev); > -void drm_unplug_dev(struct drm_device *dev); > > int drm_dev_set_unique(struct drm_device *dev, const char *name); > > -- > 2.1.4 > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-04-12 15:03 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-12 8:56 [PATCH v9 0/1] drm: rockchip: Fix rockchip drm unbind crash error Jeffy Chen 2017-04-12 8:56 ` [PATCH v9] drm: Unplug drm device when unregistering it Jeffy Chen 2017-04-12 15:03 ` Sean Paul [this message] 2017-04-12 15:03 ` Sean Paul 2017-04-13 1:28 ` jeffy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170412150321.uqlbrkb55chajylq@art_vandelay \ --to=seanpaul@chromium.org \ --cc=airlied@linux.ie \ --cc=airlied@redhat.com \ --cc=briannorris@chromium.org \ --cc=chris@chris-wilson.co.uk \ --cc=daniel.vetter@intel.com \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=hshi@chromium.org \ --cc=jani.nikula@linux.intel.com \ --cc=jeffy.chen@rock-chips.com \ --cc=linux-kernel@vger.kernel.org \ --cc=marcheu@chromium.org \ --cc=mark.yao@rock-chips.com \ --cc=teg@jklm.no \ --cc=tfiga@chromium.org \ --cc=zyw@rock-chips.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.