[PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[Maarten Lankhorst]

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.

Changes since v1:
- Also check for state->event->base.completion being set, to
  handle the case where stall_checks() fails in setup_crtc_commit().

Fixes: 24835e442f28 ("drm: reference count event->completion")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.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 b16f1d69a0bb..1d43f3e85a7d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3327,6 +3327,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 && state->event->base.completion)
+			drm_crtc_commit_put(state->commit);
+
 		kfree(state->commit->event);
 		state->commit->event = NULL;
 		drm_crtc_commit_put(state->commit);
-- 
2.15.1

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

✓ Fi.CI.BAT: success for drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[Patchwork]

== Series Details ==

Series: drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.
URL   : https://patchwork.freedesktop.org/series/36263/
State : success

== Summary ==

Series 36263v1 drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.
https://patchwork.freedesktop.org/api/1.0/series/36263/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (fi-hsw-4770r) fdo#100368
Test kms_frontbuffer_tracking:
        Subgroup basic:
                incomplete -> PASS       (fi-bsw-n3050) fdo#104571

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#104571 https://bugs.freedesktop.org/show_bug.cgi?id=104571

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:420s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:376s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:484s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:276s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:507s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:395s
fi-hsw-4770r     total:288  pass:260  dwarn:0   dfail:0   fail:1   skip:27  time:394s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:413s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:455s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:464s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:497s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:503s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:578s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:429s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:528s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:494s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:520s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:403s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y2        total:288  pass:258  dwarn:3   dfail:0   fail:0   skip:27  time:530s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:466s

17fd16225c4df784ff06467c8bc528bee156a274 drm-tip: 2018y-01m-10d-09h-37m-51s UTC integration manifest
a41ad7e6ea78 drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7640/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[Patchwork]

== Series Details ==

Series: drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.
URL   : https://patchwork.freedesktop.org/series/36263/
State : success

== Summary ==

Warning: bzip CI_DRM_3618/shard-glkb6/results30.json.bz2 wasn't in correct JSON format
Test gem_tiled_swapping:
        Subgroup non-threaded:
                dmesg-warn -> PASS       (shard-hsw) fdo#104218 +1
Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-snb) fdo#104058
Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                notrun     -> INCOMPLETE (shard-hsw) fdo#103540 +1
        Subgroup busy-flip:
                pass       -> INCOMPLETE (shard-snb) fdo#104566 +3
Test gem_softpin:
        Subgroup noreloc-s4:
                skip       -> FAIL       (shard-hsw) fdo#103375 +1
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-hsw) fdo#100047

fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#104566 https://bugs.freedesktop.org/show_bug.cgi?id=104566
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-hsw        total:2545 pass:1446 dwarn:1   dfail:1   fail:9   skip:1084 time:8003s
shard-snb        total:2576 pass:1244 dwarn:2   dfail:0   fail:10  skip:1318 time:7226s
Blacklisted hosts:
shard-apl        total:2576 pass:1599 dwarn:1   dfail:0   fail:24  skip:950 time:12448s
shard-kbl        total:2576 pass:1712 dwarn:2   dfail:0   fail:24  skip:836 time:9629s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7640/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[Alex Deucher]

On Wed, Jan 10, 2018 at 6:00 AM, Maarten Lankhorst
<maarten.lankhorst@linux.intel.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.
>
> Changes since v1:
> - Also check for state->event->base.completion being set, to
>   handle the case where stall_checks() fails in setup_crtc_commit().
>
> Fixes: 24835e442f28 ("drm: reference count event->completion")

Thanks for fixing this up.  You mentioned on IRC that this version
still caused problems.  What were those?

Thanks,

Alex

> Cc: <stable@vger.kernel.org> # v4.11+
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
> Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.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 b16f1d69a0bb..1d43f3e85a7d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3327,6 +3327,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 && state->event->base.completion)
> +                       drm_crtc_commit_put(state->commit);
> +
>                 kfree(state->commit->event);
>                 state->commit->event = NULL;
>                 drm_crtc_commit_put(state->commit);
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel