Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Leo Li <sunpeng.li@amd.com>,
	dri-devel@lists.freedesktop.org,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Zuo, Jerry" <Jerry.Zuo@amd.com>,
	Harry Wentland <harry.wentland@amd.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: Non-blocking commits on -ERESTARTSYS
Date: Wed, 13 Dec 2017 18:23:37 +0100	[thread overview]
Message-ID: <19f321ab-8896-f746-3d75-e50e680b8c00@linux.intel.com> (raw)
In-Reply-To: <018d7df5-15cb-38f5-8ade-53db7b197c1a@amd.com>

[-- 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

       reply	other threads:[~2017-12-13 17:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <018d7df5-15cb-38f5-8ade-53db7b197c1a@amd.com>
2017-12-13 17:23 ` Maarten Lankhorst [this message]
2017-12-13 19:24   ` Non-blocking commits on -ERESTARTSYS Leo Li
2017-12-14 14:43     ` Leo Li
2017-12-13 17:55 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-13 19:43 ` ✗ Fi.CI.IGT: failure " Patchwork

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=19f321ab-8896-f746-3d75-e50e680b8c00@linux.intel.com \
    --to=maarten.lankhorst@linux.intel.com \
    --cc=Jerry.Zuo@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=harry.wentland@amd.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=sunpeng.li@amd.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