Hi,
On 30-05-17 14:02, Hans de Goede wrote:
Hi,
On 29-05-17 21:02, Daniel Vetter wrote:
On Sun, May 28, 2017 at 07:16:55PM +0200, Hans de Goede wrote:
Since commit a39be606f99d ("drm: Do a full device unregister when unplugging") drm_unplug_dev has been calling drm_dev_unregister followed by a drm_put_dev when open_count reaches 0. This drm_put_dev calls drm_dev_unregister again. Since drm_dev_unregister is not protected against being called multiple times this leads to havoc.
This commit fixes this by calling drm_dev_unref instead of drm_put_dev.
Fixes: a39be606f99d ("drm: Do a full device unregister when unplugging") Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Marco Diego Aurélio Mesquita marcodiegomesquita@gmail.com Reported-by: Marco Diego Aurélio Mesquita marcodiegomesquita@gmail.com Signed-off-by: Hans de Goede hdegoede@redhat.com
Note I don't have any USB display devices at hand for testing atm so this patch has only been compile tested.
Changes in v2: -Remove unnecessary mutex changes
Offending patch is in 4.8 ... do we need cc: stable?
Yes I think a Cc: stable would be good.
Does this need a bugreport link?
There is no bug-report that I know of (Marco reported the issues to me by email). This likely fixes some crashes when unplugging USB display link devices.
Note I plan to actually test this patch with an USB display device tomorrow, so you may want to hold on merging this till I've got around to testing this.
So as promised I've been testing (and then debugging) this today, this patch is no good.
Chris was right when he said maybe we should not call drm_dev_unregister() at all, that indeed is bad and causes oopses and an unusable system on unplug. Reverting Chris' original commit replaces those oopses with different oopses and still results in an unusable system. I believe that I know how to fix this now, but it is almost 10pm over here and I think it is better if I continue this tomorrow.
In the mean time in case it was not clear: self-NACK for this patch.
Regards,
Hans
Regards,
Hans
drivers/gpu/drm/drm_drv.c | 6 +++--- drivers/gpu/drm/drm_file.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index b5c6bb46a425..30b5382bf877 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -364,9 +364,9 @@ void drm_unplug_dev(struct drm_device *dev) drm_device_set_unplugged(dev);
- if (dev->open_count == 0) {
drm_put_dev(dev);- }
- if (dev->open_count == 0)
drm_dev_unref(dev);mutex_unlock(&drm_global_mutex);diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 3783b659cd38..edba71c8ccc3 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -424,7 +424,7 @@ int drm_release(struct inode *inode, struct file *filp) if (!--dev->open_count) { drm_lastclose(dev); if (drm_device_is_unplugged(dev))
drm_put_dev(dev);
drm_dev_unref(dev); } mutex_unlock(&drm_global_mutex);-- 2.13.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel