From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 529A8C433E0 for ; Wed, 3 Feb 2021 10:43:32 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E2A7464F2C for ; Wed, 3 Feb 2021 10:43:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E2A7464F2C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=IhWh7ElVXvIaNcGGjkgH5KeLqJgp2D6O7O9l4FremX4=; b=pplNBO0pzhDkJuCoPZUv4eIof pXeDAxKyoW6Z3zltaUG+F56P7SK4dyLDBzvDjqumFatSfq5SXLJhRkbTKL40tPuG2S/pRKHTUqCbl Kli9kZrW8wCDXMMjjNuWFQqSXYUpdvh7ns3s2qyMEYMtpx9yz7ReMQCUt5OU45lpjaIV1M+okbXBa 6fpKN3xDWdX2nCpzeh3g2W1l7j9PusicdibYPpNG8n79Lfz0qSGUTmz3Tb64u7kSOuCyiYcyho6LN ObujeIxs/7oFD+1Vs5lfzs/pptfBWe/7dPciFGoDO07s069xg3fxLd6Pwz8zQZDPHPZF2BBMhUAMy aSY6ABNZg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l7Fby-0005Qk-1q; Wed, 03 Feb 2021 10:42:14 +0000 Received: from mail-wm1-x32c.google.com ([2a00:1450:4864:20::32c]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l7Fbv-0005Pz-KV for linux-arm-kernel@lists.infradead.org; Wed, 03 Feb 2021 10:42:12 +0000 Received: by mail-wm1-x32c.google.com with SMTP id j11so4152897wmi.3 for ; Wed, 03 Feb 2021 02:42:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=zOUCxvKfNLh9LXY5HAhXD0ADP7iay2Tk0x7+iDeR2XY=; b=UmKeC0lviQEflCNxco15aoGnRyrd0ymp8yVsir7RnaZ6GdDbH7bdbkj+yCfh7fuS7Z IZKxLxh2zgKuVFyymVuKOxL89XZGH3ywNw9RDhFaVvDMelRV/ydFgN4xJLGpq+QJAaDJ x3/iwRQCmTxUNm364An6c77JfICf4bN9nWQk8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=zOUCxvKfNLh9LXY5HAhXD0ADP7iay2Tk0x7+iDeR2XY=; b=fmTIQojf7ef+sksfkCUrBLAq/k93agzAZLCdr674qHaMcH+J8622u0LISkb+4D9/Ad APjVvYAiZJfguBkcYE8YF+evLoy8asssgIA0YcCWUr55Ah9TXbXY8LKFK7kWXspN2bqC gko8UaH410QRmelfKDnqvVTbTl20dz6bcFmtOge4Rs1TpZlXQuJB3mSO3LhvrR3ykHNH 5oIaP9UlND24K93WConBA+eQxmtjPBFeBchaTiQOHmmtEcDUzFTlkMrUq9bVneShf1WG AJhRj/lTSelGhVW6GziudWnJbR7BSjGeZRIeZPNJ78dA4jtAJySwHSsQkFAY3IbwXtw2 bMFA== X-Gm-Message-State: AOAM530FLyBbM3q2qybSE2N72+1B/n6+7/vCcmv0+eQ9NcqYp3HyQeSv fGxGcYpoyMe5AqS8PdeqS6KEnQ== X-Google-Smtp-Source: ABdhPJxA/ksjmhexnFn7txkr6EHLKnN9HIdUdnHVkgqUS6cdKoMgPe24RwLJ1+AoMN5G1m3vTsOkcQ== X-Received: by 2002:a05:600c:21cb:: with SMTP id x11mr2220918wmj.29.1612348930296; Wed, 03 Feb 2021 02:42:10 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id c9sm3282269wrw.76.2021.02.03.02.42.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Feb 2021 02:42:09 -0800 (PST) Date: Wed, 3 Feb 2021 11:42:07 +0100 From: Daniel Vetter To: quanyang.wang@windriver.com Subject: Re: [PATCH] drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable Message-ID: Mail-Followup-To: quanyang.wang@windriver.com, Hyun Kwon , Laurent Pinchart , David Airlie , Michal Simek , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20210202064121.173362-1-quanyang.wang@windriver.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210202064121.173362-1-quanyang.wang@windriver.com> X-Operating-System: Linux phenom 5.7.0-1-amd64 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210203_054211_744466_407F49B5 X-CRM114-Status: GOOD ( 31.45 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hyun Kwon , David Airlie , Michal Simek , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Laurent Pinchart , Daniel Vetter , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Feb 02, 2021 at 02:41:21PM +0800, quanyang.wang@windriver.com wrote: > From: Quanyang Wang > > 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 Yeah that's the way to do it, not the hack. Thanks for your patch, I'll push it to drm-misc-fixes with Cc: stable@vger.kernel.org Cheers, Daniel > --- > drivers/gpu/drm/xlnx/zynqmp_disp.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c > index 68cc4ffff969..ee7657a48e6a 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c > @@ -1396,19 +1396,11 @@ static void zynqmp_disp_enable(struct zynqmp_disp *disp) > */ > static void zynqmp_disp_disable(struct zynqmp_disp *disp) > { > - struct drm_crtc *crtc = &disp->crtc; > - > zynqmp_disp_audio_disable(&disp->audio); > > zynqmp_disp_avbuf_disable_audio(&disp->avbuf); > zynqmp_disp_avbuf_disable_channels(&disp->avbuf); > zynqmp_disp_avbuf_disable(&disp->avbuf); > - > - /* 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; > - } > } > > static inline struct zynqmp_disp *crtc_to_disp(struct drm_crtc *crtc) > @@ -1499,6 +1491,13 @@ zynqmp_disp_crtc_atomic_disable(struct drm_crtc *crtc, > > drm_crtc_vblank_off(&disp->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); > + > clk_disable_unprepare(disp->pclk); > pm_runtime_put_sync(disp->dev); > } > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel