merged.

Bruce

In message: [linux-yocto][v5.15/standard/preempt-rt/sdkv5.15/xlnx-soc][PATCH] 
drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable
on 05/09/2022 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
> [Quanyang: Since Xilinx SDK is using downstream drm driver and the upstream
> patches are missing, so we need to bring this patch back.]
> ---
> Hi Bruce,
> Would you please help merge this patch to the branches:
>       v5.15/standard/preempt-rt/sdkv5.15/xlnx-soc
>       v5.15/standard/sdkv5.15/xlnx-soc
> Thanks,
> Quanyang
> ---
>  drivers/gpu/drm/xlnx/zynqmp_disp.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c 
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 3dc772034436b..13440f542cd21 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -2041,8 +2041,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;
>  
> @@ -2051,12 +2049,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,6 +2628,14 @@ zynqmp_disp_crtc_atomic_disable(struct drm_crtc *crtc,
>       zynqmp_disp_disable(disp, true);
>       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 (#11630): 
https://lists.yoctoproject.org/g/linux-yocto/message/11630
Mute This Topic: https://lists.yoctoproject.org/mt/93471502/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to