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 <quanyang.w...@windriver.com> > > commit a7e02f7796c163ac8297b30223bf24bade9f8a50 upstream > > When running xrandr to change resolution of DP, the kmemleak as below > can be observed: > > unreferenced object 0xffff00080a351000 (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: > [<00000000e0bd0f69>] kmemleak_alloc+0x30/0x40 > [<00000000cde2f318>] kmem_cache_alloc+0x3d4/0x588 > [<0000000088ea9bd7>] drm_atomic_helper_setup_commit+0x84/0x5f8 > [<000000002290a264>] drm_atomic_helper_commit+0x58/0x388 > [<00000000f6ea78c3>] drm_atomic_commit+0x4c/0x60 > [<00000000c8e0725e>] drm_atomic_connector_commit_dpms+0xe8/0x110 > [<0000000020ade187>] drm_mode_obj_set_property_ioctl+0x1b0/0x450 > [<00000000918206d6>] drm_connector_property_set_ioctl+0x3c/0x68 > [<000000008d51e7a5>] drm_ioctl_kernel+0xc4/0x118 > [<000000002a819b75>] drm_ioctl+0x214/0x448 > [<000000008ca4e588>] __arm64_sys_ioctl+0xa8/0xf0 > [<0000000034e15a35>] el0_svc_common.constprop.0+0x74/0x190 > [<000000001b93d916>] do_el0_svc+0x24/0x90 > [<00000000ce9230e0>] el0_svc+0x14/0x20 > [<00000000e3607d82>] el0_sync_handler+0xb0/0xb8 > [<000000003e79c15f>] 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 = &commit->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 <quanyang.w...@windriver.com> > Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch> > 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 = &disp->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(&disp->av_buf); > zynqmp_disp_av_buf_disable(&disp->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, &disp->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(&crtc->dev->event_lock); > + if (crtc->state->event) { > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + crtc->state->event = NULL; > + } > + spin_unlock_irq(&crtc->dev->event_lock); > + } > pm_runtime_put_sync(disp->dev); > } > > -- > 2.36.1 >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#12831): https://lists.yoctoproject.org/g/linux-yocto/message/12831 Mute This Topic: https://lists.yoctoproject.org/mt/99844248/21656 Group Owner: linux-yocto+ow...@lists.yoctoproject.org Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/leave/6687884/21656/624485779/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-