From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout-p-103.mailbox.org (mout-p-103.mailbox.org [80.241.56.161]) (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 AF2F025A625; Wed, 9 Apr 2025 12:57:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=80.241.56.161 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744203444; cv=none; b=ZctkEI68uTKjaG1N01f5qj2Sq2MLvRPqGuWcaN1kEkWOxYVeYahiX2K4GBq2I2fvdfmkP36s0KYI0cfLkNZqcfJp+UNAwCbaL9u83FF+SaCKNCrCwYMOjhkCwZLFCx5I+e1LvVdzfKYlkMXCWWy6BCB8dRJpM3i9zwx0AgnYz8M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744203444; c=relaxed/simple; bh=Tq5WAuk54cs3/N97rx9htWFxoF+txtk1JT/rPAcwMQE=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=c5U8APfMDfUMTvTckC+xQ0zLc7vKTf3S87eqX4KNKgZW6Ym87i4NCwWmXqKUs08Ld0prkyRQEV2HSJJcRWLhtGFLBbnRwBIE/80w+kZAKcmlCq09IilWBGhSElJwNeFsimpNdiDP8WZ9RDnB3MaLI9UtM5sdYPy4/lkeqynR0kw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org; spf=pass smtp.mailfrom=mailbox.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b=ZGoZo2ZB; arc=none smtp.client-ip=80.241.56.161 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mailbox.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b="ZGoZo2ZB" Received: from smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-103.mailbox.org (Postfix) with ESMTPS id 4ZXjdp6qNnz9ss3; Wed, 9 Apr 2025 14:57:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org; s=mail20150812; t=1744203438; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Tq5WAuk54cs3/N97rx9htWFxoF+txtk1JT/rPAcwMQE=; b=ZGoZo2ZBuYYbkCYzDI8SCaoFxp2d+HcsIBxZz603lXaxsaHk1ztKwnM8dS1+ygPDAFxa0o PC/g1QZjB8Lrjh4S+Nk/fs1WdHrJwAXaEab2IUIL4T5c1Ww/GMPkP6qvXfxNp/LrcId3OT hX3RbJgjljfzAh7NvI1OgJRDglsE/2vPtC52qUWYU0Y+WJGBGnDzCIGEzgCZdI/nA6hFMI oqIcw+CFTyp3BO2nfdeAVhSjt7g8CrU+ecBLZpx5ptMCjGjZn4n8lbP12qhjCUhDcIruzG 4f+J4JRRX7z+Y0PCwkQUQ30na91K+8YIJ8GtSDqKkLH/J7c/A9uaazfM2G7lYw== Message-ID: <72eb974dfea8fa1167cf97e29848672223f6fc5b.camel@mailbox.org> Subject: Re: [PATCH 1/2] dma-fence: Rename dma_fence_is_signaled() From: Philipp Stanner Reply-To: phasta@kernel.org To: phasta@kernel.org, Boris Brezillon Cc: Sumit Semwal , Gustavo Padovan , Christian =?ISO-8859-1?Q?K=F6nig?= , Felix Kuehling , Alex Deucher , Xinhui Pan , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Lucas Stach , Russell King , Christian Gmeiner , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Tvrtko Ursulin , Frank Binns , Matt Coster , Qiang Yu , Rob Clark , Sean Paul , Konrad Dybcio , Abhinav Kumar , Dmitry Baryshkov , Marijn Suijten , Lyude Paul , Danilo Krummrich , Rob Herring , Steven Price , Dave Airlie , Gerd Hoffmann , Matthew Brost , Huang Rui , Matthew Auld , Melissa Wen , =?ISO-8859-1?Q?Ma=EDra?= Canal , Zack Rusin , Broadcom internal kernel review list , Lucas De Marchi , Thomas =?ISO-8859-1?Q?Hellstr=F6m?= , Bas Nieuwenhuizen , Yang Wang , Jesse Zhang , Tim Huang , Sathishkumar S , Saleemkhan Jamadar , Sunil Khatri , Lijo Lazar , Hawking Zhang , Ma Jun , Yunxiang Li , Eric Huang , Asad Kamal , Srinivasan Shanmugam , Jack Xiao , Friedrich Vock , Michel =?ISO-8859-1?Q?D=E4nzer?= , Geert Uytterhoeven , Anna-Maria Behnsen , Thomas Gleixner , Frederic Weisbecker , Dan Carpenter , linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org, etnaviv@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, lima@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, nouveau@lists.freedesktop.org, virtualization@lists.linux.dev, spice-devel@lists.freedesktop.org, intel-xe@lists.freedesktop.org Date: Wed, 09 Apr 2025 14:56:58 +0200 In-Reply-To: <73d41cd84c73b296789b654e45125bfce88e0dbf.camel@mailbox.org> References: <20250409120640.106408-2-phasta@kernel.org> <20250409120640.106408-3-phasta@kernel.org> <20250409143917.31303d22@collabora.com> <73d41cd84c73b296789b654e45125bfce88e0dbf.camel@mailbox.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-MBO-RS-META: qn5jmk7am4aitmyfzw96fhabh5raicqw X-MBO-RS-ID: 069014a3f81ce6e0501 On Wed, 2025-04-09 at 14:51 +0200, Philipp Stanner wrote: > On Wed, 2025-04-09 at 14:39 +0200, Boris Brezillon wrote: > > Hi Philipp, > >=20 > > On Wed,=C2=A0 9 Apr 2025 14:06:37 +0200 > > Philipp Stanner wrote: > >=20 > > > dma_fence_is_signaled()'s name strongly reads as if this function > > > were > > > intended for checking whether a fence is already signaled. Also > > > the > > > boolean it returns hints at that. > > >=20 > > > The function's behavior, however, is more complex: it can check > > > with a > > > driver callback whether the hardware's sequence number indicates > > > that > > > the fence can already be treated as signaled, although the > > > hardware's / > > > driver's interrupt handler has not signaled it yet. If that's the > > > case, > > > the function also signals the fence. > > >=20 > > > (Presumably) this has caused a bug in Nouveau (unknown commit), > > > where > > > nouveau_fence_done() uses the function to check a fence, which > > > causes a > > > race. > > >=20 > > > Give the function a more obvious name. > >=20 > > This is just my personal view on this, but I find the new name just > > as > > confusing as the old one. It sounds like something is checked, but > > it's > > clear what, and then the fence is forcibly signaled like it would > > be > > if > > you call drm_fence_signal(). Of course, this clarified by the doc, > > but > > given the goal was to make the function name clearly reflect what > > it > > does, I'm not convinced it's significantly better. > >=20 > > Maybe dma_fence_check_hw_state_and_propagate(), though it might be > > too long of name. Oh well, feel free to ignore this comments if a > > majority is fine with the new name. >=20 > Yoa, the name isn't perfect (the perfect name describing the whole > behavior would be > dma_fence_check_if_already_signaled_then_check_hardware_state_and_pro > pa > gate() ^^' >=20 > My intention here is to have the reader realize "watch out, the fence > might get signaled here!", which is probably the most important event > regarding fences, which can race, invoke the callbacks and so on. >=20 > For details readers will then check the documentation. >=20 > But I'm of course open to see if there's a majority for this or that > name. how about: dma_fence_check_hw_and_signal() ? P. >=20 > P. >=20 >=20 > >=20 > > Regards, > >=20 > > Boris >=20