From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
During a non-blocking commit, it is possible to return before the commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending vblank event when preparing the commit, the above situation will leave us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the end of a commit, release it's reference.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ab40321..4253f57 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) { if (state->commit) { + /* + * In the event that a non-blocking commit returns + * -ERESTARTSYS before the commit_tail work is queued, we will + * have an extra reference to the commit object. Release it, if + * the event has not been consumed by the worker. + */ + if (state->event) + drm_crtc_commit_put(state->commit); + kfree(state->commit->event); state->commit->event = NULL; drm_crtc_commit_put(state->commit);
On 2018-01-04 02:47 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
During a non-blocking commit, it is possible to return before the commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending vblank event when preparing the commit, the above situation will leave us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the end of a commit, release it's reference.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
No expert on this but looks sane to me.
Acked-by: Harry Wentland harry.wentland@amd.com
Harry
drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ab40321..4253f57 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) { if (state->commit) {
/** In the event that a non-blocking commit returns* -ERESTARTSYS before the commit_tail work is queued, we will* have an extra reference to the commit object. Release it, if* the event has not been consumed by the worker.*/if (state->event)drm_crtc_commit_put(state->commit);- kfree(state->commit->event); state->commit->event = NULL; drm_crtc_commit_put(state->commit);
On Mon, Jan 8, 2018 at 4:30 PM, Harry Wentland harry.wentland@amd.com wrote:
On 2018-01-04 02:47 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
During a non-blocking commit, it is possible to return before the commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending vblank event when preparing the commit, the above situation will leave us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the end of a commit, release it's reference.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
No expert on this but looks sane to me.
Acked-by: Harry Wentland harry.wentland@amd.com
Pushed to drm-misc-next.
Thanks,
Alex
Harry
drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ab40321..4253f57 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) { if (state->commit) {
/** In the event that a non-blocking commit returns* -ERESTARTSYS before the commit_tail work is queued, we will* have an extra reference to the commit object. Release it, if* the event has not been consumed by the worker.*/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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jan 09, 2018 at 01:13:33PM -0500, Alex Deucher wrote:
On Mon, Jan 8, 2018 at 4:30 PM, Harry Wentland harry.wentland@amd.com wrote:
On 2018-01-04 02:47 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
During a non-blocking commit, it is possible to return before the commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending vblank event when preparing the commit, the above situation will leave us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the end of a commit, release it's reference.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
No expert on this but looks sane to me.
Acked-by: Harry Wentland harry.wentland@amd.com
Pushed to drm-misc-next.
This blows up all over the place in our CI:
https://bugs.freedesktop.org/show_bug.cgi?id=104566
I think as a leak fix it should also be pushed to -fixes. And we didn't really figure out what's wrong, so we're going with the quick revert until you folks are all awake.
Cheers, Daniel
Thanks,
Alex
Harry
drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ab40321..4253f57 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) { if (state->commit) {
/** In the event that a non-blocking commit returns* -ERESTARTSYS before the commit_tail work is queued, we will* have an extra reference to the commit object. Release it, if* the event has not been consumed by the worker.*/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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Op 08-01-18 om 22:30 schreef Harry Wentland:
On 2018-01-04 02:47 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
During a non-blocking commit, it is possible to return before the commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending vblank event when preparing the commit, the above situation will leave us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the end of a commit, release it's reference.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
No expert on this but looks sane to me.
Acked-by: Harry Wentland harry.wentland@amd.com
Harry
Hey,
I've reverted the patch for now. I planned to apply it to the drm-misc-fixes branch with the appropriate commits referenced, but it is causing crashes on the kms_flip testcase.
https://bugs.freedesktop.org/show_bug.cgi?id=104566
I'll investigate it some more then send out a fixed patch. :)
~Maarten
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Daniel Vetter -
Harry Wentland -
Maarten Lankhorst -
sunpeng.li＠amd.com