All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Bobroff <sbobroff@linux.ibm.com>
To: kraxel@redhat.com, airlied@linux.ie, daniel@ffwll.ch,
	virtualization@lists.linux-foundation.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/1] drm/bochs: Fix connector leak during driver unload
Date: Tue, 21 May 2019 15:28:39 +1000	[thread overview]
Message-ID: <93b363ad62f4938d9ddf3e05b2a61e3f66b2dcd3.1558416473.git.sbobroff@linux.ibm.com> (raw)

When unloading the bochs-drm driver, a warning message is printed by
drm_mode_config_cleanup() because a reference is still held to one of
the drm_connector structs.

Correct this by calling drm_atomic_helper_shutdown() in
bochs_pci_remove().

The issue is caused by the interaction of two previous commits. Both
together are required to cause it:
Fixes: 846c7dfc1193 ("drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.")
Fixes: 6579c39594ae ("drm/bochs: atomic: switch planes to atomic, wire up helpers.")

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
Hello,

This seems to be similar to an issue I recently fixed for the AST driver:
1e613f3c630c ("drm/ast: Fix connector leak during driver unload")
Which is similar to at least one other recent fix:
192b4af6cd28 ("drm/tegra: Shutdown on driver unbind")

The fix seems to be the same, but this time I've tried to dig a little deeper
and use an appropriate Fixes tag to assist backporting.

Bisecting the issue for commits to drivers/gpu/drm/bochs/ points to:
6579c39594ae ("drm/bochs: atomic: switch planes to atomic, wire up helpers.")
... but the issue also seems to be due to a change in the generic drm code
(reverting it separately fixes the issue):
846c7dfc1193 ("drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.")
... so I've included both in the commit.  Is that the right thing to do?

I couldn't help wondering if we should also update the comment for
drm_fbdev_generic_setup(), because it says:
"The fbdev is destroyed by drm_dev_unregister()"
... which implies to me that cleanup only requires that call, but actually
since 846c7dfc1193 you will always(?) need to use drm_atomic_helper_shutdown()
as well. (Is it actually always the case?)

Cheers,
Sam.

 drivers/gpu/drm/bochs/bochs_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c
index 6b6e037258c3..7031f0168795 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -10,6 +10,7 @@
 #include <linux/slab.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 #include "bochs.h"
 
@@ -174,6 +175,7 @@ static void bochs_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
+	drm_atomic_helper_shutdown(dev);
 	drm_dev_unregister(dev);
 	bochs_unload(dev);
 	drm_dev_put(dev);
-- 
2.19.0.2.gcad72f5712


             reply	other threads:[~2019-05-21  5:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-21  5:28 Sam Bobroff [this message]
2019-05-21  8:10 ` [PATCH 1/1] drm/bochs: Fix connector leak during driver unload Gerd Hoffmann
2019-05-21  8:10 ` Gerd Hoffmann
2019-06-17  1:20   ` [EXTERNAL] " Sam Bobroff
2019-06-17  1:20     ` Sam Bobroff
2019-06-17  5:45     ` Gerd Hoffmann
2019-06-17  5:45       ` Gerd Hoffmann
  -- strict thread matches above, loose matches on Subject: below --
2019-05-21  5:28 Sam Bobroff

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=93b363ad62f4938d9ddf3e05b2a61e3f66b2dcd3.1558416473.git.sbobroff@linux.ibm.com \
    --to=sbobroff@linux.ibm.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /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: link
Be 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.