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=-6.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,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 AE786C32750 for ; Fri, 2 Aug 2019 10:01:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7C1C72073D for ; Fri, 2 Aug 2019 10:01:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="SUOcFbPy" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407123AbfHBKBx (ORCPT ); Fri, 2 Aug 2019 06:01:53 -0400 Received: from mail-oi1-f196.google.com ([209.85.167.196]:41269 "EHLO mail-oi1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2405130AbfHBJnw (ORCPT ); Fri, 2 Aug 2019 05:43:52 -0400 Received: by mail-oi1-f196.google.com with SMTP id g7so56358152oia.8 for ; Fri, 02 Aug 2019 02:43:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=A+FPU/vGw66DQH6ItIezOEVqjuBIxZcPX9USWgAb1jM=; b=SUOcFbPy5rxss1N6dYEveiIIOMKAwymDOUtmfTGkpGgQ/Xpr3UWjMsAcCCDkBnqdup 1eq2a1R6v6DN0QkaPATZN5n0kE4ue5wnZyoBJTKNOqWawLO3d/A97+DsCdyNlpGcjyQZ XE6laiEawO4BJz5p9iAOjzFnHPln42mGtcH2k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=A+FPU/vGw66DQH6ItIezOEVqjuBIxZcPX9USWgAb1jM=; b=JfDDPrWi/wN4boRyRqx9Y0JlFuTlj1y5P5aVZGsS1Lu8WFNtqhCtwV0ATLwCWqHL9O 2OywW4KvegL/7K/uTSZo9oTDM9KcA9rExdrdnbueAo7b+082m54jBlyhhBtdnhdsEfzz hfFA0AANzINHy02VweSa+9bmM/sk9Fh7HI6TTUPWMfwd6jTkI9tMR2k0os9nrHn8rq7D rRtMb2tl/ifvbl5qtz/LwsuQsc5EWkpl00dsjiHBUy6ofjPzWvRiNGGK9te/a4rf2Rtt +OYZzDR+eVql1Q0lWrSe9f2CWH4Ntwxf4olHhwhRmT/Ox1Q4IaIAwAeNvStE7gFGObf2 UpNw== X-Gm-Message-State: APjAAAUO5/Um9eE6P6/N7nG8zIajxWOJm3eBGpTcmQ1kaeW3SuBE2jWw NV7fEMOnsmJQv/nE4h+JFaJS24K1vDtHfazxqfOANg== X-Google-Smtp-Source: APXvYqwrE1bn5ct0WaJ9P1S7sbbbgAxT5gZWv30BGTYJ1sG1vdgqFLWcTAJ+BcSNC7gWOTWkKRRExPmjXgf4HAnSNP0= X-Received: by 2002:aca:b104:: with SMTP id a4mr2199449oif.14.1564739031061; Fri, 02 Aug 2019 02:43:51 -0700 (PDT) MIME-Version: 1.0 References: <1564571048-15029-1-git-send-email-lowry.li@arm.com> <1564571048-15029-3-git-send-email-lowry.li@arm.com> <20190731132002.dut5mdsqgh7b75iv@DESKTOP-E1NTVVP.localdomain> <20190802092920.4la5cwrltv2m6dke@DESKTOP-E1NTVVP.localdomain> In-Reply-To: <20190802092920.4la5cwrltv2m6dke@DESKTOP-E1NTVVP.localdomain> From: Daniel Vetter Date: Fri, 2 Aug 2019 11:43:39 +0200 Message-ID: Subject: Re: [PATCH v1 2/2] drm: Clear the fence pointer when writeback job signaled To: Brian Starkey Cc: "Lowry Li (Arm Technology China)" , Liviu Dudau , "james qian wang (Arm Technology China)" , "maarten.lankhorst@linux.intel.com" , "seanpaul@chromium.org" , "airlied@linux.ie" , "Julien Yin (Arm Technology China)" , "maxime.ripard@bootlin.com" , "eric@anholt.net" , "kieran.bingham+renesas@ideasonboard.com" , "sean@poorly.run" , "laurent.pinchart@ideasonboard.com" , "Jonathan Chai (Arm Technology China)" , Ayan Halder , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "linux-renesas-soc@vger.kernel.org" , nd Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 2, 2019 at 11:29 AM Brian Starkey wrote: > > Hi Lowry, > > On Thu, Aug 01, 2019 at 06:34:08AM +0000, Lowry Li (Arm Technology China) wrote: > > Hi Brian, > > > > On Wed, Jul 31, 2019 at 09:20:04PM +0800, Brian Starkey wrote: > > > Hi Lowry, > > > > > > Thanks for this cleanup. > > > > > > On Wed, Jul 31, 2019 at 11:04:45AM +0000, Lowry Li (Arm Technology China) wrote: > > > > During it signals the completion of a writeback job, after releasing > > > > the out_fence, we'd clear the pointer. > > > > > > > > Check if fence left over in drm_writeback_cleanup_job(), release it. > > > > > > > > Signed-off-by: Lowry Li (Arm Technology China) > > > > --- > > > > drivers/gpu/drm/drm_writeback.c | 23 +++++++++++++++-------- > > > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > > > > index ff138b6..43d9e3b 100644 > > > > --- a/drivers/gpu/drm/drm_writeback.c > > > > +++ b/drivers/gpu/drm/drm_writeback.c > > > > @@ -324,6 +324,9 @@ void drm_writeback_cleanup_job(struct drm_writeback_job *job) > > > > if (job->fb) > > > > drm_framebuffer_put(job->fb); > > > > > > > > + if (job->out_fence) > > > > > > I'm thinking it might be a good idea to signal the fence with an error > > > here, if it's not already signaled. Otherwise, if there's someone > > > waiting (which there shouldn't be), they're going to be waiting a very > > > long time :-) > > > > > > Thanks, > > > -Brian > > > > > Here it happened at atomic_check failed and test only commit. For both > > cases, the commit has been dropped and it's only a clean up. So here better > > not be treated as an error case:) > > If anyone else has a reference on the fence, then IMO it absolutely is > an error to reach this point without the fence being signaled - > because it means that the fence will never be signaled. > > I don't think the API gives you a way to check if this is the last > reference, so it's safest to just make sure the fence is signalled > before dropping the reference. > > It just feels wrong to me to have the possibility of a dangling fence > which is never going to get signalled; and it's an easy defensive step > to make sure it can never happen. > > I know it _shouldn't_ happen, but we often put in handling for cases > which shouldn't happen, because they frequently do happen :-) We're not as paranoid with the vblank fences either, so not sure why we need to be this paranoid with writeback fences. If your driver grabs anything from the atomic state in ->atomic_check it's buggy anyway. If you want to fix this properly I think we need to move the call to prepare_signalling() in between atomic_check and atomic_commit. Then I think it makes sense to also force-complete the fence on error ... -Daniel > > Since for userspace, it should have been failed or a test only case, so > > writebace fence should not be signaled. > > It's not only userspace that can wait on fences (and in fact this > fence will never even reach userspace if the commit fails), the driver > may have taken a copy to use for "something". > > Cheers, > -Brian > > > > > Best regards, > > Lowry > > > > + dma_fence_put(job->out_fence); > > > > + > > > > kfree(job); > > > > } > > > > -- > > Regards, > > Lowry -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v1 2/2] drm: Clear the fence pointer when writeback job signaled Date: Fri, 2 Aug 2019 11:43:39 +0200 Message-ID: References: <1564571048-15029-1-git-send-email-lowry.li@arm.com> <1564571048-15029-3-git-send-email-lowry.li@arm.com> <20190731132002.dut5mdsqgh7b75iv@DESKTOP-E1NTVVP.localdomain> <20190802092920.4la5cwrltv2m6dke@DESKTOP-E1NTVVP.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-oi1-x243.google.com (mail-oi1-x243.google.com [IPv6:2607:f8b0:4864:20::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id EC9276ED93 for ; Fri, 2 Aug 2019 09:43:51 +0000 (UTC) Received: by mail-oi1-x243.google.com with SMTP id g7so56358154oia.8 for ; Fri, 02 Aug 2019 02:43:51 -0700 (PDT) In-Reply-To: <20190802092920.4la5cwrltv2m6dke@DESKTOP-E1NTVVP.localdomain> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Starkey Cc: "linux-renesas-soc@vger.kernel.org" , nd , Ayan Halder , "airlied@linux.ie" , Liviu Dudau , "Jonathan Chai (Arm Technology China)" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "Julien Yin (Arm Technology China)" , "maxime.ripard@bootlin.com" , "kieran.bingham+renesas@ideasonboard.com" , "james qian wang (Arm Technology China)" , "laurent.pinchart@ideasonboard.com" , "seanpaul@chromium.org" , "Lowry Li (Arm Technology China)" , "sean@poorly.run" List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBBdWcgMiwgMjAxOSBhdCAxMToyOSBBTSBCcmlhbiBTdGFya2V5IDxCcmlhbi5TdGFy a2V5QGFybS5jb20+IHdyb3RlOgo+Cj4gSGkgTG93cnksCj4KPiBPbiBUaHUsIEF1ZyAwMSwgMjAx OSBhdCAwNjozNDowOEFNICswMDAwLCBMb3dyeSBMaSAoQXJtIFRlY2hub2xvZ3kgQ2hpbmEpIHdy b3RlOgo+ID4gSGkgQnJpYW4sCj4gPgo+ID4gT24gV2VkLCBKdWwgMzEsIDIwMTkgYXQgMDk6MjA6 MDRQTSArMDgwMCwgQnJpYW4gU3RhcmtleSB3cm90ZToKPiA+ID4gSGkgTG93cnksCj4gPiA+Cj4g PiA+IFRoYW5rcyBmb3IgdGhpcyBjbGVhbnVwLgo+ID4gPgo+ID4gPiBPbiBXZWQsIEp1bCAzMSwg MjAxOSBhdCAxMTowNDo0NUFNICswMDAwLCBMb3dyeSBMaSAoQXJtIFRlY2hub2xvZ3kgQ2hpbmEp IHdyb3RlOgo+ID4gPiA+IER1cmluZyBpdCBzaWduYWxzIHRoZSBjb21wbGV0aW9uIG9mIGEgd3Jp dGViYWNrIGpvYiwgYWZ0ZXIgcmVsZWFzaW5nCj4gPiA+ID4gdGhlIG91dF9mZW5jZSwgd2UnZCBj bGVhciB0aGUgcG9pbnRlci4KPiA+ID4gPgo+ID4gPiA+IENoZWNrIGlmIGZlbmNlIGxlZnQgb3Zl ciBpbiBkcm1fd3JpdGViYWNrX2NsZWFudXBfam9iKCksIHJlbGVhc2UgaXQuCj4gPiA+ID4KPiA+ ID4gPiBTaWduZWQtb2ZmLWJ5OiBMb3dyeSBMaSAoQXJtIFRlY2hub2xvZ3kgQ2hpbmEpIDxsb3dy eS5saUBhcm0uY29tPgo+ID4gPiA+IC0tLQo+ID4gPiA+ICBkcml2ZXJzL2dwdS9kcm0vZHJtX3dy aXRlYmFjay5jIHwgMjMgKysrKysrKysrKysrKysrLS0tLS0tLS0KPiA+ID4gPiAgMSBmaWxlIGNo YW5nZWQsIDE1IGluc2VydGlvbnMoKyksIDggZGVsZXRpb25zKC0pCj4gPiA+ID4KPiA+ID4gPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV93cml0ZWJhY2suYyBiL2RyaXZlcnMvZ3B1 L2RybS9kcm1fd3JpdGViYWNrLmMKPiA+ID4gPiBpbmRleCBmZjEzOGI2Li40M2Q5ZTNiIDEwMDY0 NAo+ID4gPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fd3JpdGViYWNrLmMKPiA+ID4gPiAr KysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX3dyaXRlYmFjay5jCj4gPiA+ID4gQEAgLTMyNCw2ICsz MjQsOSBAQCB2b2lkIGRybV93cml0ZWJhY2tfY2xlYW51cF9qb2Ioc3RydWN0IGRybV93cml0ZWJh Y2tfam9iICpqb2IpCj4gPiA+ID4gICBpZiAoam9iLT5mYikKPiA+ID4gPiAgICAgICAgICAgZHJt X2ZyYW1lYnVmZmVyX3B1dChqb2ItPmZiKTsKPiA+ID4gPgo+ID4gPiA+ICsgaWYgKGpvYi0+b3V0 X2ZlbmNlKQo+ID4gPgo+ID4gPiBJJ20gdGhpbmtpbmcgaXQgbWlnaHQgYmUgYSBnb29kIGlkZWEg dG8gc2lnbmFsIHRoZSBmZW5jZSB3aXRoIGFuIGVycm9yCj4gPiA+IGhlcmUsIGlmIGl0J3Mgbm90 IGFscmVhZHkgc2lnbmFsZWQuIE90aGVyd2lzZSwgaWYgdGhlcmUncyBzb21lb25lCj4gPiA+IHdh aXRpbmcgKHdoaWNoIHRoZXJlIHNob3VsZG4ndCBiZSksIHRoZXkncmUgZ29pbmcgdG8gYmUgd2Fp dGluZyBhIHZlcnkKPiA+ID4gbG9uZyB0aW1lIDotKQo+ID4gPgo+ID4gPiBUaGFua3MsCj4gPiA+ IC1Ccmlhbgo+ID4gPgo+ID4gSGVyZSBpdCBoYXBwZW5lZCBhdCBhdG9taWNfY2hlY2sgZmFpbGVk IGFuZCB0ZXN0IG9ubHkgY29tbWl0LiBGb3IgYm90aAo+ID4gY2FzZXMsIHRoZSBjb21taXQgaGFz IGJlZW4gZHJvcHBlZCBhbmQgaXQncyBvbmx5IGEgY2xlYW4gdXAuIFNvIGhlcmUgYmV0dGVyCj4g PiBub3QgYmUgdHJlYXRlZCBhcyBhbiBlcnJvciBjYXNlOikKPgo+IElmIGFueW9uZSBlbHNlIGhh cyBhIHJlZmVyZW5jZSBvbiB0aGUgZmVuY2UsIHRoZW4gSU1PIGl0IGFic29sdXRlbHkgaXMKPiBh biBlcnJvciB0byByZWFjaCB0aGlzIHBvaW50IHdpdGhvdXQgdGhlIGZlbmNlIGJlaW5nIHNpZ25h bGVkIC0KPiBiZWNhdXNlIGl0IG1lYW5zIHRoYXQgdGhlIGZlbmNlIHdpbGwgbmV2ZXIgYmUgc2ln bmFsZWQuCj4KPiBJIGRvbid0IHRoaW5rIHRoZSBBUEkgZ2l2ZXMgeW91IGEgd2F5IHRvIGNoZWNr IGlmIHRoaXMgaXMgdGhlIGxhc3QKPiByZWZlcmVuY2UsIHNvIGl0J3Mgc2FmZXN0IHRvIGp1c3Qg bWFrZSBzdXJlIHRoZSBmZW5jZSBpcyBzaWduYWxsZWQKPiBiZWZvcmUgZHJvcHBpbmcgdGhlIHJl ZmVyZW5jZS4KPgo+IEl0IGp1c3QgZmVlbHMgd3JvbmcgdG8gbWUgdG8gaGF2ZSB0aGUgcG9zc2li aWxpdHkgb2YgYSBkYW5nbGluZyBmZW5jZQo+IHdoaWNoIGlzIG5ldmVyIGdvaW5nIHRvIGdldCBz aWduYWxsZWQ7IGFuZCBpdCdzIGFuIGVhc3kgZGVmZW5zaXZlIHN0ZXAKPiB0byBtYWtlIHN1cmUg aXQgY2FuIG5ldmVyIGhhcHBlbi4KPgo+IEkga25vdyBpdCBfc2hvdWxkbid0XyBoYXBwZW4sIGJ1 dCB3ZSBvZnRlbiBwdXQgaW4gaGFuZGxpbmcgZm9yIGNhc2VzCj4gd2hpY2ggc2hvdWxkbid0IGhh cHBlbiwgYmVjYXVzZSB0aGV5IGZyZXF1ZW50bHkgZG8gaGFwcGVuIDotKQoKV2UncmUgbm90IGFz IHBhcmFub2lkIHdpdGggdGhlIHZibGFuayBmZW5jZXMgZWl0aGVyLCBzbyBub3Qgc3VyZSB3aHkK d2UgbmVlZCB0byBiZSB0aGlzIHBhcmFub2lkIHdpdGggd3JpdGViYWNrIGZlbmNlcy4gSWYgeW91 ciBkcml2ZXIKZ3JhYnMgYW55dGhpbmcgZnJvbSB0aGUgYXRvbWljIHN0YXRlIGluIC0+YXRvbWlj X2NoZWNrIGl0J3MgYnVnZ3kKYW55d2F5LgoKSWYgeW91IHdhbnQgdG8gZml4IHRoaXMgcHJvcGVy bHkgSSB0aGluayB3ZSBuZWVkIHRvIG1vdmUgdGhlIGNhbGwgdG8KcHJlcGFyZV9zaWduYWxsaW5n KCkgaW4gYmV0d2VlbiBhdG9taWNfY2hlY2sgYW5kIGF0b21pY19jb21taXQuIFRoZW4gSQp0aGlu ayBpdCBtYWtlcyBzZW5zZSB0byBhbHNvIGZvcmNlLWNvbXBsZXRlIHRoZSBmZW5jZSBvbiBlcnJv ciAuLi4KLURhbmllbAoKPiA+IFNpbmNlIGZvciB1c2Vyc3BhY2UsIGl0IHNob3VsZCBoYXZlIGJl ZW4gZmFpbGVkIG9yIGEgdGVzdCBvbmx5IGNhc2UsIHNvCj4gPiB3cml0ZWJhY2UgZmVuY2Ugc2hv dWxkIG5vdCBiZSBzaWduYWxlZC4KPgo+IEl0J3Mgbm90IG9ubHkgdXNlcnNwYWNlIHRoYXQgY2Fu IHdhaXQgb24gZmVuY2VzIChhbmQgaW4gZmFjdCB0aGlzCj4gZmVuY2Ugd2lsbCBuZXZlciBldmVu IHJlYWNoIHVzZXJzcGFjZSBpZiB0aGUgY29tbWl0IGZhaWxzKSwgdGhlIGRyaXZlcgo+IG1heSBo YXZlIHRha2VuIGEgY29weSB0byB1c2UgZm9yICJzb21ldGhpbmciLgo+Cj4gQ2hlZXJzLAo+IC1C cmlhbgo+Cj4gPgo+ID4gQmVzdCByZWdhcmRzLAo+ID4gTG93cnkKPiA+ID4gPiArICAgICAgICAg ZG1hX2ZlbmNlX3B1dChqb2ItPm91dF9mZW5jZSk7Cj4gPiA+ID4gKwo+ID4gPiA+ICAga2ZyZWUo am9iKTsKPiA+ID4gPiAgfQo+ID4KPiA+IC0tCj4gPiBSZWdhcmRzLAo+ID4gTG93cnkKCgoKLS0g CkRhbmllbCBWZXR0ZXIKU29mdHdhcmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9uCis0MSAo MCkgNzkgMzY1IDU3IDQ4IC0gaHR0cDovL2Jsb2cuZmZ3bGwuY2gKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmkt ZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs