dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>,
	Mika Kuoppala <mika.kuoppala@linux.intel.com>,
	Andi Shyti <andi.shyti@intel.com>,
	Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>,
	Matthew Auld <matthew.auld@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: [PATCH 08/51] drm/i915: Use drmm_add_final_kfree
Date: Mon,  2 Mar 2020 23:25:48 +0100	[thread overview]
Message-ID: <20200302222631.3861340-9-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <20200302222631.3861340-1-daniel.vetter@ffwll.ch>

With this we can drop the final kfree from the release function.

The mock device in the selftests needed it's pci_device split
up from the drm_device. In the future we could simplify this again
by allocating the pci_device as a managed allocation too.

v2: I overlooked that i915_driver_destroy is also called in the
unwind code of the error path. There we need a drm_dev_put.
Similar for the mock object.

Now the problem with that is that the drm_driver->release callbacks
for both the real driver and the mock one assume everything has been
set up. Hence going through that path for a partially set up driver
will result in issues. Quickest fix is to disable the ->release() hook
until the driver is fully initialized, and keep the onion unwinding.
Long term would be cleanest to move everything over to drmm_ release
actions, but that's a lot of work for a big driver like i915. Plus
more core work needed first anyway.

v3: Fix i915_drm pointer wrangling in mock_gem_device. Also switch
over to start using drm_dev_put() to clean up even on the error path.
Aside I think the current error path is leaking the allocation.

v4: more fixes for intel-gfx-ci, some if it damage from v3 :-/

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_drv.c               | 10 ++++++-
 drivers/gpu/drm/i915/i915_drv.h               |  3 ++
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 30 ++++++++++++++-----
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5997b4507f29..bb14209beeed 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -43,6 +43,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_irq.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_probe_helper.h>
 
 #include "display/intel_acpi.h"
@@ -895,6 +896,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return ERR_PTR(err);
 	}
 
+	drmm_add_final_kfree(&i915->drm, i915);
+
 	i915->drm.pdev = pdev;
 	pci_set_drvdata(pdev, i915);
 
@@ -913,7 +916,6 @@ static void i915_driver_destroy(struct drm_i915_private *i915)
 	struct pci_dev *pdev = i915->drm.pdev;
 
 	drm_dev_fini(&i915->drm);
-	kfree(i915);
 }
 
 /**
@@ -997,6 +999,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915_welcome_messages(i915);
 
+	i915->do_release = true;
+
 	return 0;
 
 out_cleanup_irq:
@@ -1017,6 +1021,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 out_fini:
 	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
 	i915_driver_destroy(i915);
+	drm_dev_put(&i915->drm);
 	return ret;
 }
 
@@ -1056,6 +1061,9 @@ static void i915_driver_release(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
 
+	if (!dev_priv->do_release)
+		return;
+
 	disable_rpm_wakeref_asserts(rpm);
 
 	i915_gem_driver_release(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b621df933212..865865ca3b42 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -884,6 +884,9 @@ struct i915_selftest_stash {
 struct drm_i915_private {
 	struct drm_device drm;
 
+	/* FIXME: Device release actions should all be moved to drmm_ */
+	bool do_release;
+
 	const struct intel_device_info __info; /* Use INTEL_INFO() to access. */
 	struct intel_runtime_info __runtime; /* Use RUNTIME_INFO() to access. */
 	struct intel_driver_caps caps;
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 754d0eb6beaa..acf889e4b993 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -25,6 +25,8 @@
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 
+#include <drm/drm_managed.h>
+
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_requests.h"
 #include "gt/mock_engine.h"
@@ -55,6 +57,9 @@ static void mock_device_release(struct drm_device *dev)
 {
 	struct drm_i915_private *i915 = to_i915(dev);
 
+	if (!i915->do_release)
+		goto out;
+
 	mock_device_flush(i915);
 	intel_gt_driver_remove(&i915->gt);
 
@@ -72,7 +77,9 @@ static void mock_device_release(struct drm_device *dev)
 	drm_mode_config_cleanup(&i915->drm);
 
 	drm_dev_fini(&i915->drm);
+out:
 	put_device(&i915->drm.pdev->dev);
+	i915->drm.pdev = NULL;
 }
 
 static struct drm_driver mock_driver = {
@@ -114,9 +121,14 @@ struct drm_i915_private *mock_gem_device(void)
 	struct pci_dev *pdev;
 	int err;
 
-	pdev = kzalloc(sizeof(*pdev) + sizeof(*i915), GFP_KERNEL);
+	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
 	if (!pdev)
-		goto err;
+		return NULL;
+	i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
+	if (!i915) {
+		kfree(pdev);
+		return NULL;
+	}
 
 	device_initialize(&pdev->dev);
 	pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
@@ -129,7 +141,6 @@ struct drm_i915_private *mock_gem_device(void)
 	pdev->dev.archdata.iommu = (void *)-1;
 #endif
 
-	i915 = (struct drm_i915_private *)(pdev + 1);
 	pci_set_drvdata(pdev, i915);
 
 	dev_pm_domain_set(&pdev->dev, &pm_domain);
@@ -141,9 +152,13 @@ struct drm_i915_private *mock_gem_device(void)
 	err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
 	if (err) {
 		pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-		goto put_device;
+		put_device(&pdev->dev);
+		kfree(i915);
+
+		return NULL;
 	}
 	i915->drm.pdev = pdev;
+	drmm_add_final_kfree(&i915->drm, i915);
 
 	intel_runtime_pm_init_early(&i915->runtime_pm);
 
@@ -188,6 +203,8 @@ struct drm_i915_private *mock_gem_device(void)
 	__clear_bit(I915_WEDGED, &i915->gt.reset.flags);
 	intel_engines_driver_register(i915);
 
+	i915->do_release = true;
+
 	return i915;
 
 err_context:
@@ -199,8 +216,7 @@ struct drm_i915_private *mock_gem_device(void)
 	intel_memory_regions_driver_release(i915);
 	drm_mode_config_cleanup(&i915->drm);
 	drm_dev_fini(&i915->drm);
-put_device:
-	put_device(&pdev->dev);
-err:
+	drm_dev_put(&i915->drm);
+
 	return NULL;
 }
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-03-02 22:27 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 22:25 [PATCH 00/51] drm_device managed resources, v4 Daniel Vetter
2020-03-02 22:25 ` [PATCH 01/51] mm/sl[uo]b: export __kmalloc_track(_node)_caller Daniel Vetter
2020-03-02 22:25 ` [PATCH 02/51] drm/i915: Don't clear drvdata in ->release Daniel Vetter
2020-03-02 22:25 ` [PATCH 03/51] drm: add managed resources tied to drm_device Daniel Vetter
2020-03-03  8:04   ` Dan Carpenter
2020-03-03  8:25     ` Daniel Vetter
2020-03-03  8:44   ` [PATCH] " Daniel Vetter
2020-03-06 19:38     ` Sam Ravnborg
2020-03-11  9:07   ` [PATCH 03/51] " Thomas Zimmermann
2020-03-11  9:47     ` Thomas Zimmermann
2020-03-16  8:45     ` Daniel Vetter
2020-03-11  9:14   ` Thomas Zimmermann
2020-03-16  8:50     ` Daniel Vetter
2020-03-02 22:25 ` [PATCH 04/51] drm: Set final_kfree in drm_dev_alloc Daniel Vetter
2020-03-03  7:44   ` Gerd Hoffmann
2020-03-07  8:06   ` Sam Ravnborg
2020-03-23 11:02     ` Daniel Vetter
2020-03-11  9:09   ` Thomas Zimmermann
2020-03-02 22:25 ` [PATCH 05/51] drm/mipi_dbi: Use drmm_add_final_kfree in all drivers Daniel Vetter
2020-03-02 22:25 ` [PATCH 06/51] drm/udl: Use drmm_add_final_kfree Daniel Vetter
2020-03-11  9:10   ` Thomas Zimmermann
2020-03-02 22:25 ` [PATCH 07/51] drm/qxl: " Daniel Vetter
2020-03-03  7:44   ` Gerd Hoffmann
2020-03-02 22:25 ` Daniel Vetter [this message]
2020-03-02 22:25 ` [PATCH 09/51] drm/cirrus: " Daniel Vetter
2020-03-03  7:49   ` Gerd Hoffmann
2020-03-03  8:27     ` Daniel Vetter
2020-03-02 22:25 ` [PATCH 10/51] drm/v3d: " Daniel Vetter
2020-03-02 22:25 ` [PATCH 11/51] drm/tidss: " Daniel Vetter
2020-03-02 22:25 ` [PATCH 12/51] drm/mcde: " Daniel Vetter
2020-03-02 22:25 ` [PATCH 13/51] drm/vgem: " Daniel Vetter
2020-03-02 22:25 ` [PATCH 14/51] drm/vkms: " Daniel Vetter
2020-03-02 22:25 ` [PATCH 15/51] drm/repaper: " Daniel Vetter
2020-03-06 20:27   ` Sam Ravnborg
2020-03-02 22:25 ` [PATCH 16/51] drm/ingenic: " Daniel Vetter
2020-03-06 20:27   ` Sam Ravnborg
2020-03-02 22:25 ` [PATCH 17/51] drm/gm12u320: " Daniel Vetter
2020-03-06 20:27   ` Sam Ravnborg
2020-03-02 22:25 ` [PATCH 18/51] drm/<drivers>: " Daniel Vetter
2020-03-06 20:28   ` Sam Ravnborg
2020-03-02 22:25 ` [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout Daniel Vetter
2020-03-03  8:45   ` [PATCH] " Daniel Vetter
2020-03-06 20:31     ` Sam Ravnborg
2020-03-06 20:29   ` [PATCH 19/51] " Sam Ravnborg
2020-03-11  9:15   ` Thomas Zimmermann
2020-03-02 22:26 ` [PATCH 20/51] drm: Handle dev->unique with drmm_ Daniel Vetter
2020-03-06 20:37   ` Sam Ravnborg
2020-03-23 10:54     ` Daniel Vetter
2020-03-11  9:19   ` Thomas Zimmermann
2020-03-02 22:26 ` [PATCH 21/51] drm: Use drmm_ for drm_dev_init cleanup Daniel Vetter
2020-03-11  9:39   ` Thomas Zimmermann
2020-03-16  9:02     ` Daniel Vetter
2020-03-02 22:26 ` [PATCH 22/51] drm: manage drm_minor cleanup with drmm_ Daniel Vetter
2020-03-11  9:59   ` Thomas Zimmermann
2020-03-16  9:07     ` Daniel Vetter
2020-03-02 22:26 ` [PATCH 23/51] drm: Manage drm_gem_init " Daniel Vetter
2020-03-07  8:20   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 24/51] drm: Manage drm_vblank_cleanup " Daniel Vetter
2020-03-07  8:28   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 25/51] drm: Garbage collect drm_dev_fini Daniel Vetter
2020-03-07  8:30   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 26/51] drm: Manage drm_mode_config_init with drmm_ Daniel Vetter
2020-03-06 20:04   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 27/51] drm/bochs: Remove leftover drm_atomic_helper_shutdown Daniel Vetter
2020-03-03  7:49   ` Gerd Hoffmann
2020-03-02 22:26 ` [PATCH 28/51] drm/bochs: Drop explicit drm_mode_config_cleanup Daniel Vetter
2020-03-03  7:50   ` Gerd Hoffmann
2020-03-06 20:14   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 29/51] drm/cirrus: Drop explicit drm_mode_config_cleanup call Daniel Vetter
2020-03-03  7:51   ` Gerd Hoffmann
2020-03-02 22:26 ` [PATCH 30/51] drm/cirrus: Fully embrace devm_ Daniel Vetter
2020-03-03  7:51   ` Gerd Hoffmann
2020-03-02 22:26 ` [PATCH 31/51] drm/ingenic: Drop explicit drm_mode_config_cleanup call Daniel Vetter
2020-03-06 20:17   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 32/51] drm/mcde: " Daniel Vetter
2020-03-06 20:18   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 33/51] drm/mcde: More devm_drm_dev_init Daniel Vetter
2020-03-02 22:26 ` [PATCH 34/51] drm/meson: Drop explicit drm_mode_config_cleanup call Daniel Vetter
2020-03-02 22:26 ` [PATCH 35/51] drm/pl111: " Daniel Vetter
2020-03-07  8:37   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 36/51] drm/rcar-du: " Daniel Vetter
2020-03-02 22:26 ` [PATCH 37/51] drm/rockchip: " Daniel Vetter
2020-03-07  8:38   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 38/51] drm/stm: " Daniel Vetter
2020-03-07  9:25   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 39/51] drm/shmob: " Daniel Vetter
2020-03-02 22:26 ` [PATCH 40/51] drm/mtk: " Daniel Vetter
2020-03-07  9:26   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 41/51] drm/tidss: " Daniel Vetter
2020-03-02 22:26 ` [PATCH 42/51] drm/gm12u320: More drmm_ Daniel Vetter
2020-03-02 22:26 ` [PATCH 43/51] drm/gm12u320: Use devm_drm_dev_init Daniel Vetter
2020-03-02 22:26 ` [PATCH 44/51] drm/gm12u320: Use helpers for shutdown/suspend/resume Daniel Vetter
2020-03-02 22:26 ` [PATCH 45/51] drm/gm12u320: Simplify upload work Daniel Vetter
2020-03-02 22:26 ` [PATCH 46/51] drm/repaper: Drop explicit drm_mode_config_cleanup call Daniel Vetter
2020-03-07  9:31   ` Sam Ravnborg
2020-03-02 22:26 ` [PATCH 47/51] drm/mipi-dbi: Move drm_mode_config_init into mipi library Daniel Vetter
2020-03-02 22:26 ` [PATCH 48/51] drm/mipi-dbi: Drop explicit drm_mode_config_cleanup call Daniel Vetter
2020-03-02 22:26 ` [PATCH 49/51] drm/udl: " Daniel Vetter
2020-03-02 22:26 ` [PATCH 50/51] drm/udl: drop drm_driver.release hook Daniel Vetter
2020-03-03  7:57   ` Thomas Zimmermann
2020-03-02 22:26 ` [PATCH 51/51] drm: Add docs for managed resources Daniel Vetter
2020-03-07 10:07   ` Sam Ravnborg
  -- strict thread matches above, loose matches on Subject: below --
2020-03-23 14:48 [PATCH 00/51] drm_device managed resources, v5 Daniel Vetter
2020-03-23 14:49 ` [PATCH 08/51] drm/i915: Use drmm_add_final_kfree Daniel Vetter
2020-03-26 13:10   ` Jani Nikula
2020-03-26 13:33     ` Daniel Vetter
2020-02-27 18:14 [PATCH 00/51] drm managed resources, v3 Daniel Vetter
2020-02-27 18:14 ` [PATCH 08/51] drm/i915: Use drmm_add_final_kfree Daniel Vetter
2020-02-21 21:02 [PATCH 00/51] drm managed resources, v2 Daniel Vetter
2020-02-21 21:02 ` [PATCH 08/51] drm/i915: Use drmm_add_final_kfree Daniel Vetter

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=20200302222631.3861340-9-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=abdiel.janulgue@linux.intel.com \
    --cc=andi.shyti@intel.com \
    --cc=daniel.vetter@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=mika.kuoppala@linux.intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=tvrtko.ursulin@intel.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).