On 2017-12-13 12:23 PM, Maarten Lankhorst wrote:
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?
Good point, it seems sane to me. I gave it a spin and it fixes the issue.
I was concerned that it'll contend with the worker thread, possibly
freeing the crtc_commit before the flip is done. It seems the
atomic_state_get before the work is queued will prevent that.
Leo
No idea if sane though, but drivers are supposed to clear crtc_state->event on success..
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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
