Re: Non-blocking commits on -ERESTARTSYS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2017-12-13 02:24 PM, Leo Li wrote:


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

Hi Maarten,

If there are no objections, can I submit a patch with your change below?

Thanks,
Leo


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);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux