From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7998C1C695 for ; Mon, 15 Jun 2026 11:11:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.97.179.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781521921; cv=none; b=t1HnX31a6WsSgnUDn1NLTEauIDegt7PpI82Loq91a04ps37IxNyAeaHV8qERksiGlxa76CSdLZNC76o7NYELURbVxBuG2amihIuGa+hijkVB8rzcgk51L513zUHWFN9dFSKmyZ0EjdzmNYr6N4+F3DmwrC4CKEA09G8G2rqSc9Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781521921; c=relaxed/simple; bh=pqhLZtqlkbqScbG/Lukcr0bnGmcLRn30pibYXIwMOtU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=f0Vnahg7yEB2cyvOTrD1EHKh5fZU5M4YDVGvrQp/2gCc41yKSUaEIXKYUvet65koIetTWbZp2uzc3u2b4YK4xp/enYRXsX8mxYdSiLHiOP52cb4Re9SlknqPL4FnWmwT9Ghb7Q7TFedWy/0J2Hk9jXRncsXYaqbpM7oPKxAW3pc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com; spf=pass smtp.mailfrom=igalia.com; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b=LK9Id9nZ; arc=none smtp.client-ip=213.97.179.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=igalia.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b="LK9Id9nZ" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=J8z5/sBeVOpFzThk6XKqFMnyqBylHs8OOOz6fH6jtSs=; b=LK9Id9nZVRTEVSssYLvokciwVF BmeDksX8NLhe6ZIf2XcuE5640CK+bd7DozUbOoclTOnIMy/E7RtjBLksHWEwKowsr0OXn/mx3UQWn /sXmgIHX8zEGm0rSR6zWLotak07zwZlXZqFtiFZKVXZUDztW7jW5ypzi48CCaGqc3wF0s9AFjNGqi ITQZNMvwSKWU7xPYtRxWprhCMCatddtamFprgedV+58N5pWrYF5idbvJKBAbRScjJGJWxMBTbQygV 0EzVoUXb/1znnnaTM+vCRqekQQ6vH2NAlF4gcSlMlJvo2gqnDqOcFjcp81BfpZfQ6k7bKbzbsyOmz kOpUkZBQ==; Received: from [90.240.106.137] (helo=[192.168.0.116]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1wZ5EA-000CQq-Qf; Mon, 15 Jun 2026 13:11:38 +0200 Message-ID: <3834b1d6-c78f-45b8-aa12-d464a770747c@igalia.com> Date: Mon, 15 Jun 2026 12:11:37 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH] dma-buf/dma_fence: Make races for dma_fence_is_signaled() less likely To: Philipp Stanner , Danilo Krummrich , Maarten Lankhorst , David Airlie , Simona Vetter , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= , Boris Brezillon , "Paul E . McKenney" Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260612104251.2264707-2-phasta@kernel.org> Content-Language: en-GB From: Tvrtko Ursulin In-Reply-To: <20260612104251.2264707-2-phasta@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 12/06/2026 11:42, Philipp Stanner wrote: > dma_fence_is_signaled() returns whether a fence has been signaled > already. That function contains a fast path opportunistic check which is > not guarded by the lock and, according to Christian, cannot be guarded > by the lock without causing a massive performance regression. > > This now means that dma_fence_is_signaled() can return true WHILE the > fence callbacks are still being executed. This is razy and has lead to > at least one bug solved in: > > commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race") > > Make this race impossible, by simply setting the bit only once the > callbacks are actually completed. My 2c is that I would be wary of trying to turn around the semantics of the callbacks. They are not "run these to make the fence signaled" but instead "let me know please when fence is signaled". I do not see a good justification for the change. The bugs such as above quoted c8a5d5ea3ba6 are in fact more about nouveau itself pulling data underneath fences with callbacks and not a bug in dma-fence. With the opportunistic cleanup it is doing in nouveau_cli_work() perhaps it would want to use dma_fence_remove_callback() to appear somewhat sensible, although that too may be racy, I did not think about it. Anyway, I've sent a patch to make that one clearer and more documented, even faster for the unsignaled case. Regards, Tvrtko > > Signed-off-by: Philipp Stanner > --- > drivers/dma-buf/dma-fence.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index c7ea1e75d38a..2416cc86ce93 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -359,8 +359,19 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence, > > dma_fence_assert_held(fence); > > - if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, > - &fence->flags))) > + /* > + * First test the bit, so we don't signal an already signaled fence again. > + * The lock protects against multiple parties setting the bit. The bit > + * is then set at the end of the function. > + * > + * The background is that there is a fast path check in > + * dma_fence_is_signaled() which does not use lock protection and can > + * return true *while* the fence callbacks are still executing. > + * > + * This fast path check supposedly cannot be guarded by the lock because > + * of significant performance regressions. > + */ > + if (unlikely(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))) > return; > > trace_dma_fence_signaled(fence); > @@ -384,6 +395,9 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence, > INIT_LIST_HEAD(&cur->node); > cur->func(fence, cur); > } > + > + // TODO: we need some barrier here, don't we? > + set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags); > } > EXPORT_SYMBOL(dma_fence_signal_timestamp_locked); >