Re: Non-blocking commits on -ERESTARTSYS

* Re: Non-blocking commits on -ERESTARTSYS
       [not found] <018d7df5-15cb-38f5-8ade-53db7b197c1a@amd.com>
@ 2017-12-13 17:23 ` Maarten Lankhorst
  2017-12-13 19:24   ` Leo Li
  2017-12-13 17:55 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-12-13 19:43 ` ✗ Fi.CI.IGT: failure " Patchwork
  2 siblings, 1 reply; 5+ messages in thread
From: Maarten Lankhorst @ 2017-12-13 17:23 UTC (permalink / raw)
  To: Leo Li, dri-devel, Daniel Vetter
  Cc: Zuo, Jerry, Harry Wentland, Intel Graphics Development

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

Op 13-12-17 om 17:19 schreef Leo Li:
> Hi Daniel, Maarten,
>
> Just digging an old thread out of the grave:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150495.html
>
> It's suppose to fix a memory leak on the drm_commit object during
> non-blocking commits. Within drm_atomic_helper_setup_commit, a reference
> to the commit object is obtained by the new_crtc_state. This reference
> is suppose to be 'put' once flip_done is signaled (via the
> release_crtc_commit callback), but never happens if .prepare_fb returns
> -ERESTARTSYS: drm_atomic_helper_commit early returns before the
> commit_tail work is queued.
>
> We're starting to bump into this issue again. Regarding Daniel's
> suggestion for an IGT test, has there been any work done on it? I'd be
> interested in taking a look otherwise. As a side note, I can also
> reproduce this on i915.
>
> Thanks,
> Leo

I'm curious, isn't it better to handle this in __drm_atomic_helper_crtc_destroy_state with the attached patch?

No idea if sane though, but drivers are supposed to clear crtc_state->event on success..


[-- Attachment #2: drop-extra-refcount.patch --]
[-- Type: text/x-diff, Size: 553 bytes --]

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 593b30d38ce0..e71233b4c651 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3435,6 +3435,8 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
 {
 	if (state->commit) {
+		if (state->event)
+			drm_crtc_commit_put(state->commit);
 		kfree(state->commit->event);
 		state->commit->event = NULL;
 		drm_crtc_commit_put(state->commit);

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread