From patchwork Fri Aug 13 20:30:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 12436137 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=-18.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT 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 690A8C432BE for ; Fri, 13 Aug 2021 20:30:54 +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 BD4EF6103A for ; Fri, 13 Aug 2021 20:30:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BD4EF6103A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1565D6E8D6; Fri, 13 Aug 2021 20:30:45 +0000 (UTC) Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by gabe.freedesktop.org (Postfix) with ESMTPS id A1D5B6E8D1 for ; Fri, 13 Aug 2021 20:30:43 +0000 (UTC) Received: by mail-ed1-x531.google.com with SMTP id b7so17200526edu.3 for ; Fri, 13 Aug 2021 13:30:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=1tzQoJk2Vxo3bdO8JMvHymsLoOVGe9IUEBg7fmejmSM=; b=E4xfGf+kkRkDDhWKr20asokxo+TwFXhbpjwT92tHle5Tav8BBh3RJNFRc/sGl5/Qz1 +8RQ0o3IC6fJ6lci+4vo8GDKXnOfgTeG+c1B080igyQ6pnAFt4sG+9DdHhWpOMQqeLE1 rb2uCbzSfeWKXjPllWJrNvrIax2vmeit/He9E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=1tzQoJk2Vxo3bdO8JMvHymsLoOVGe9IUEBg7fmejmSM=; b=niWtyu0Ox+Qa9H4SCLgMeNvtNAlTEVkHLChTKP8XrULQB/VHXgNkL6+Ed24jvyMldl dhwdCcls6V49+qWPKU9vsiS4akXJ+8qFGLGkExt70BOxjYZ3uivn/Qjc0TKJP+tn4/8W ODSxTTjT5q6T4FAhNxAg+dJ+fPeaE2mLtIAke/6j0Qg7HLB3YvfRWzAWW2k0Len4GKZW tRUToJloPDZNy5c9lkQrk8KJMi0RmM8dOOEFiZdOjEamWuT8StDo55qX3IqaLifjYc21 RPSXtupmsE573rQHPgaXBFrqcDxqNx9qLPm2z/EW8FzoDHRGgdhOpDLazISJA76TCNiD nh3g== X-Gm-Message-State: AOAM532UXX+GObzBXEp4HYrVy1BR87jKBkRFKliUNdYQYwXSzaNMGRQL 5EuDk+lXrYztKRcHgDxuNhqp8Y+ZTw++wQ== X-Google-Smtp-Source: ABdhPJyHwb0nDIsdVRzzydLDO7+timEIsg3kAQVTYQkX0GjtKt0x6Q556BGtINF0J0mPtEmhAqGTDQ== X-Received: by 2002:a05:6402:445:: with SMTP id p5mr5385530edw.208.1628886642004; Fri, 13 Aug 2021 13:30:42 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id y17sm1347027edv.51.2021.08.13.13.30.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Aug 2021 13:30:41 -0700 (PDT) From: Daniel Vetter To: DRI Development Cc: Intel Graphics Development , Daniel Vetter , Daniel Vetter , Jason Ekstrand , Chris Wilson , Tvrtko Ursulin , Joonas Lahtinen , Matthew Brost , Matthew Auld , Maarten Lankhorst , =?utf-8?q?Thomas_Hel?= =?utf-8?q?lstr=C3=B6m?= , Lionel Landwerlin , Dave Airlie Subject: [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close Date: Fri, 13 Aug 2021 22:30:24 +0200 Message-Id: <20210813203033.3179400-2-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210813203033.3179400-1-daniel.vetter@ffwll.ch> References: <20210813203033.3179400-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" gem context refcounting is another exercise in least locking design it seems, where most things get destroyed upon context closure (which can race with anything really). Only the actual memory allocation and the locks survive while holding a reference. This tripped up Jason when reimplementing the single timeline feature in commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d Author: Jason Ekstrand Date: Thu Jul 8 10:48:12 2021 -0500 drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4) We could fix the bug by holding ctx->mutex in execbuf and clear the pointer (again while holding the mutex) context_close, but it's cleaner to just make the context object actually invariant over its _entire_ lifetime. This way any other ioctl that's potentially racing, but holding a full reference, can still rely on ctx->syncobj being an immutable pointer. Which without this change, is not the case. Signed-off-by: Daniel Vetter Fixes: 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)") Cc: Jason Ekstrand Cc: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Cc: Matthew Brost Cc: Matthew Auld Cc: Maarten Lankhorst Cc: "Thomas Hellström" Cc: Lionel Landwerlin Cc: Dave Airlie --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 051bc357ff65..5a053cf14948 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -994,6 +994,9 @@ static void i915_gem_context_release_work(struct work_struct *work) trace_i915_context_free(ctx); GEM_BUG_ON(!i915_gem_context_is_closed(ctx)); + if (ctx->syncobj) + drm_syncobj_put(ctx->syncobj); + mutex_destroy(&ctx->engines_mutex); mutex_destroy(&ctx->lut_mutex); @@ -1220,9 +1223,6 @@ static void context_close(struct i915_gem_context *ctx) if (vm) i915_vm_close(vm); - if (ctx->syncobj) - drm_syncobj_put(ctx->syncobj); - ctx->file_priv = ERR_PTR(-EBADF); /*