diff mbox series

[1/1] drm/i915: Fix memleak in runtime wakeref tracking

Message ID 20190627144438.32458-1-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/1] drm/i915: Fix memleak in runtime wakeref tracking | expand

Commit Message

Mika Kuoppala June 27, 2019, 2:44 p.m. UTC
If we untrack wakerefs, the actual count may reach zero.
However the krealloced owners array is still there and
needs to be taken care of. Free the owners unconditionally
to fix the leak.

Fixes: dbf99c1f8c7e ("drm/i915: Force printing wakeref tacking during pm_cleanup")
Reported-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Chris Wilson June 27, 2019, 2:53 p.m. UTC | #1
Quoting Mika Kuoppala (2019-06-27 15:44:38)
> If we untrack wakerefs, the actual count may reach zero.
> However the krealloced owners array is still there and
> needs to be taken care of. Free the owners unconditionally
> to fix the leak.

That is true.

> Fixes: dbf99c1f8c7e ("drm/i915: Force printing wakeref tacking during pm_cleanup")

However, the bug is mine
Fixes: bd780f37a361 ("drm/i915: Track all held rpm wakerefs")

> Reported-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Mika Kuoppala June 27, 2019, 3 p.m. UTC | #2
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-06-27 15:44:38)
>> If we untrack wakerefs, the actual count may reach zero.
>> However the krealloced owners array is still there and
>> needs to be taken care of. Free the owners unconditionally
>> to fix the leak.
>
> That is true.
>
>> Fixes: dbf99c1f8c7e ("drm/i915: Force printing wakeref tacking during pm_cleanup")
>
> However, the bug is mine
> Fixes: bd780f37a361 ("drm/i915: Track all held rpm wakerefs")

Indeed. Apologies Imre!

I personally know the reviewer of the original patch
and will have a discussion.
-Mika

>
>> Reported-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
Imre Deak June 27, 2019, 3:06 p.m. UTC | #3
On Thu, Jun 27, 2019 at 06:00:21PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-06-27 15:44:38)
> >> If we untrack wakerefs, the actual count may reach zero.
> >> However the krealloced owners array is still there and
> >> needs to be taken care of. Free the owners unconditionally
> >> to fix the leak.
> >
> > That is true.
> >
> >> Fixes: dbf99c1f8c7e ("drm/i915: Force printing wakeref tacking during pm_cleanup")
> >
> > However, the bug is mine
> > Fixes: bd780f37a361 ("drm/i915: Track all held rpm wakerefs")
> 
> Indeed. Apologies Imre!
> 
> I personally know the reviewer of the original patch
> and will have a discussion.

Good catch, I've been staring at the kmemleak logs J-P provided, but
couldn't see how it can happen. 

> -Mika
> 
> >
> >> Reported-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Imre Deak <imre.deak@intel.com>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > -Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 502c54428570..8d1aebc3e857 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -221,13 +221,11 @@  __untrack_all_wakerefs(struct intel_runtime_pm_debug *debug,
 static void
 dump_and_free_wakeref_tracking(struct intel_runtime_pm_debug *debug)
 {
-	struct drm_printer p;
+	if (debug->count) {
+		struct drm_printer p = drm_debug_printer("i915");
 
-	if (!debug->count)
-		return;
-
-	p = drm_debug_printer("i915");
-	__print_intel_runtime_pm_wakeref(&p, debug);
+		__print_intel_runtime_pm_wakeref(&p, debug);
+	}
 
 	kfree(debug->owners);
 }