Re: [linux-yocto][v6.1/standard/preempt-rt/sdkv6.1/xlnx-soc][PATCH] drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable
In message: [linux-yocto][v6.1/standard/preempt-rt/sdkv6.1/xlnx-soc][PATCH]
drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable
on 29/06/2023 quanyang.w...@windriver.com wrote:
> From: Quanyang Wang
>
> commit a7e02f7796c163ac8297b30223bf24bade9f8a50 upstream
>
> When running xrandr to change resolution of DP, the kmemleak as below
> can be observed:
>
> unreferenced object 0x00080a351000 (size 256):
> comm "Xorg", pid 248, jiffies 4294899614 (age 19.960s)
> hex dump (first 32 bytes):
> 98 a0 bc 01 08 00 ff ff 01 00 00 00 00 00 00 00
> ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00
> backtrace:
> [<e0bd0f69>] kmemleak_alloc+0x30/0x40
> [<cde2f318>] kmem_cache_alloc+0x3d4/0x588
> [<88ea9bd7>] drm_atomic_helper_setup_commit+0x84/0x5f8
> [<2290a264>] drm_atomic_helper_commit+0x58/0x388
> [<f6ea78c3>] drm_atomic_commit+0x4c/0x60
> [<c8e0725e>] drm_atomic_connector_commit_dpms+0xe8/0x110
> [<20ade187>] drm_mode_obj_set_property_ioctl+0x1b0/0x450
> [<918206d6>] drm_connector_property_set_ioctl+0x3c/0x68
> [<8d51e7a5>] drm_ioctl_kernel+0xc4/0x118
> [<2a819b75>] drm_ioctl+0x214/0x448
> [<8ca4e588>] __arm64_sys_ioctl+0xa8/0xf0
> [<34e15a35>] el0_svc_common.constprop.0+0x74/0x190
> [<1b93d916>] do_el0_svc+0x24/0x90
> [<ce9230e0>] el0_svc+0x14/0x20
> [<e3607d82>] el0_sync_handler+0xb0/0xb8
> [<3e79c15f>] el0_sync+0x174/0x180
>
> This is because there is a scenario that a drm_crtc_commit commit is
> allocated but not freed. The drm subsystem require/release references
> to a CRTC commit by calling drm_crtc_commit_get/put, and when
> drm_crtc_commit_put find that commit.ref.refcount is zero, it will
> call __drm_crtc_commit_free to free this CRTC commit. Among these
> drm_crtc_commit_get/put pairs, there is a drm_crtc_commit_get in
> drm_atomic_helper_setup_commit as below:
>
> ...
> new_crtc_state->event->base.completion = >flip_done;
> new_crtc_state->event->base.completion_release = release_crtc_commit;
> drm_crtc_commit_get(commit);
> ...
>
> This reference to the CRTC commit should be released at the function
> release_crtc_commit by calling e->completion_release(e->completion) in
> drm_send_event_locked. So we need to call drm_send_event_locked at
> two places: handling vblank event in the irq handler and the crtc disable
> helper. But in zynqmp_disp_crtc_atomic_disable, it only marks the flip
> is done and not call drm_crtc_commit_put. This result that the refcount
> of this commit is always non-zero and this commit will never be freed.
>
> Since the function drm_crtc_send_vblank_event has operations both sending
> a flip_done signal and releasing reference to the CRTC commit, let's use
> it instead.
>
> Signed-off-by: Quanyang Wang
> Signed-off-by: Daniel Vetter
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210202064121.173362-1-quanyang.w...@windriver.com
> ---
> Hi Bruce,
> Would you please help merge this patch to the branches:
> v6.1/standard/preempt-rt/sdkv6.1/xlnx-soc
> v6.1/standard/sdkv6.1/xlnx-soc
merged.
Bruce
> Thanks,
> Quanyang
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index c86e7a7775c5f..69fc2c48579fd 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -2043,8 +2043,6 @@ static void zynqmp_disp_enable(struct zynqmp_disp *disp)
> */
> static void zynqmp_disp_disable(struct zynqmp_disp *disp, bool force)
> {
> - struct drm_crtc *crtc = >xlnx_crtc.crtc;
> -
> if (!force && (!disp->enabled || zynqmp_disp_layer_is_enabled(disp)))
> return;
>
> @@ -2053,12 +2051,6 @@ static void zynqmp_disp_disable(struct zynqmp_disp
> *disp, bool force)
> zynqmp_disp_av_buf_disable_buf(>av_buf);
> zynqmp_disp_av_buf_disable(>av_buf);
>
> - /* Mark the flip is done as crtc is disabled anyway */
> - if (crtc->state->event) {
> - complete_all(crtc->state->event->base.completion);
> - crtc->state->event = NULL;
> - }
> -
> disp->enabled = false;
> }
>
> @@ -2636,8 +2628,16 @@ zynqmp_disp_crtc_atomic_disable(struct drm_crtc *crtc,
> zynqmp_disp_clk_d
[linux-yocto][v6.1/standard/preempt-rt/sdkv6.1/xlnx-soc][PATCH] drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable
From: Quanyang Wang commit a7e02f7796c163ac8297b30223bf24bade9f8a50 upstream When running xrandr to change resolution of DP, the kmemleak as below can be observed: unreferenced object 0x00080a351000 (size 256): comm "Xorg", pid 248, jiffies 4294899614 (age 19.960s) hex dump (first 32 bytes): 98 a0 bc 01 08 00 ff ff 01 00 00 00 00 00 00 00 ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmemleak_alloc+0x30/0x40 [ ] kmem_cache_alloc+0x3d4/0x588 [<88ea9bd7>] drm_atomic_helper_setup_commit+0x84/0x5f8 [<2290a264>] drm_atomic_helper_commit+0x58/0x388 [ ] drm_atomic_commit+0x4c/0x60 [ ] drm_atomic_connector_commit_dpms+0xe8/0x110 [<20ade187>] drm_mode_obj_set_property_ioctl+0x1b0/0x450 [<918206d6>] drm_connector_property_set_ioctl+0x3c/0x68 [<8d51e7a5>] drm_ioctl_kernel+0xc4/0x118 [<2a819b75>] drm_ioctl+0x214/0x448 [<8ca4e588>] __arm64_sys_ioctl+0xa8/0xf0 [<34e15a35>] el0_svc_common.constprop.0+0x74/0x190 [<1b93d916>] do_el0_svc+0x24/0x90 [ ] el0_svc+0x14/0x20 [ ] el0_sync_handler+0xb0/0xb8 [<3e79c15f>] el0_sync+0x174/0x180 This is because there is a scenario that a drm_crtc_commit commit is allocated but not freed. The drm subsystem require/release references to a CRTC commit by calling drm_crtc_commit_get/put, and when drm_crtc_commit_put find that commit.ref.refcount is zero, it will call __drm_crtc_commit_free to free this CRTC commit. Among these drm_crtc_commit_get/put pairs, there is a drm_crtc_commit_get in drm_atomic_helper_setup_commit as below: ... new_crtc_state->event->base.completion = >flip_done; new_crtc_state->event->base.completion_release = release_crtc_commit; drm_crtc_commit_get(commit); ... This reference to the CRTC commit should be released at the function release_crtc_commit by calling e->completion_release(e->completion) in drm_send_event_locked. So we need to call drm_send_event_locked at two places: handling vblank event in the irq handler and the crtc disable helper. But in zynqmp_disp_crtc_atomic_disable, it only marks the flip is done and not call drm_crtc_commit_put. This result that the refcount of this commit is always non-zero and this commit will never be freed. Since the function drm_crtc_send_vblank_event has operations both sending a flip_done signal and releasing reference to the CRTC commit, let's use it instead. Signed-off-by: Quanyang Wang Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20210202064121.173362-1-quanyang.w...@windriver.com --- Hi Bruce, Would you please help merge this patch to the branches: v6.1/standard/preempt-rt/sdkv6.1/xlnx-soc v6.1/standard/sdkv6.1/xlnx-soc Thanks, Quanyang --- drivers/gpu/drm/xlnx/zynqmp_disp.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c index c86e7a7775c5f..69fc2c48579fd 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c @@ -2043,8 +2043,6 @@ static void zynqmp_disp_enable(struct zynqmp_disp *disp) */ static void zynqmp_disp_disable(struct zynqmp_disp *disp, bool force) { - struct drm_crtc *crtc = >xlnx_crtc.crtc; - if (!force && (!disp->enabled || zynqmp_disp_layer_is_enabled(disp))) return; @@ -2053,12 +2051,6 @@ static void zynqmp_disp_disable(struct zynqmp_disp *disp, bool force) zynqmp_disp_av_buf_disable_buf(>av_buf); zynqmp_disp_av_buf_disable(>av_buf); - /* Mark the flip is done as crtc is disabled anyway */ - if (crtc->state->event) { - complete_all(crtc->state->event->base.completion); - crtc->state->event = NULL; - } - disp->enabled = false; } @@ -2636,8 +2628,16 @@ zynqmp_disp_crtc_atomic_disable(struct drm_crtc *crtc, zynqmp_disp_clk_disable(disp->pclk, >pclk_en); zynqmp_disp_plane_disable(crtc->primary); zynqmp_disp_disable(disp, true); - if (!disp->dpsub->external_crtc_attached) + if (!disp->dpsub->external_crtc_attached) { drm_crtc_vblank_off(crtc); + + spin_lock_irq(>dev->event_lock); + if (crtc->state->event) { + drm_crtc_send_vblank_event(crtc, crtc->state->event); + crtc->state->event = NULL; + } + spin_unlock_irq(>dev->event_lock); + } pm_runtime_put_sync(disp->dev); } -- 2.36.1 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#12822): https://lists.yoctoproject.org/g/linux-yocto/message/12822 Mute This Topic: https://lists.yoctoproject.org/mt/99844248/21656 Group
