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=-7.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, HK_RANDOM_FROM,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 6747DC2D0CE for ; Fri, 24 Jan 2020 09:12:41 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 36BAE2075D for ; Fri, 24 Jan 2020 09:12:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 36BAE2075D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B0A1E6FFC0; Fri, 24 Jan 2020 09:12:40 +0000 (UTC) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 005B76FFC0 for ; Fri, 24 Jan 2020 09:12:39 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Jan 2020 01:12:39 -0800 X-IronPort-AV: E=Sophos;i="5.70,357,1574150400"; d="scan'208";a="230215088" Received: from wmszyfel-mobl2.ger.corp.intel.com (HELO [10.252.10.247]) ([10.252.10.247]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/AES256-SHA; 24 Jan 2020 01:12:38 -0800 To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <20200123224459.38128-1-chris@chris-wilson.co.uk> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: <51d86897-7c11-b7f8-b9b9-f49490c62d7a@linux.intel.com> Date: Fri, 24 Jan 2020 09:12:36 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200123224459.38128-1-chris@chris-wilson.co.uk> Content-Language: en-US Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check activity on i915_vma after confirming pin_count==0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 23/01/2020 22:44, Chris Wilson wrote: > Only assert that the i915_vma is now idle if and only if no other pins > are present. If another user has the i915_vma pinned, they may submit > more work to the i915_vma skipping the vm->mutex used to serialise the > unbind. We need to wait again, if we want to continue and unbind this > vma. > > However, if we own the i915_vma (we hold the vm->mutex for the unbind > and the pin_count is 0), we can assert that the vma remains idle as we > unbind. > > Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex") > Closes: https://gitlab.freedesktop.org/drm/intel/issues/530 > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/i915_vma.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index 306b951831fe..4999882fbceb 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -1202,16 +1202,26 @@ int __i915_vma_unbind(struct i915_vma *vma) > if (ret) > return ret; > > - GEM_BUG_ON(i915_vma_is_active(vma)); > if (i915_vma_is_pinned(vma)) { > vma_print_allocator(vma, "is pinned"); > return -EAGAIN; > } > > - GEM_BUG_ON(i915_vma_is_active(vma)); > + /* > + * After confirming that no one else is pinning this vma, wait for > + * any laggards who may have crept in during the wait (through > + * a residual pin skipping the vm->mutex) to complete. > + */ > + ret = i915_vma_sync(vma); > + if (ret) > + return ret; > + > if (!drm_mm_node_allocated(&vma->node)) > return 0; > > + GEM_BUG_ON(i915_vma_is_pinned(vma)); > + GEM_BUG_ON(i915_vma_is_active(vma)); > + > if (i915_vma_is_map_and_fenceable(vma)) { > /* > * Check that we have flushed all writes through the GGTT > GEM_BUG_ON in a sandwich of two syncs, which is similar to the while loop from an earlier version. Are you sure there are no races with this one? If pinned check is the main gate then wouldn't one sync after the pinned check be enough? Especially since in 2/2 you add another sync before taking the mutex. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx