Message-ID: <20250409143917.31303d22@collabora.com>
Date: Wed, 9 Apr 2025 14:39:17 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Philipp Stanner <phasta@...nel.org>
Cc: Sumit Semwal <sumit.semwal@...aro.org>, Gustavo Padovan
 <gustavo@...ovan.org>, Christian König
 <christian.koenig@....com>, Felix Kuehling <Felix.Kuehling@....com>, Alex
 Deucher <alexander.deucher@....com>, Xinhui Pan <Xinhui.Pan@....com>, David
 Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, Lucas Stach
 <l.stach@...gutronix.de>, Russell King <linux+etnaviv@...linux.org.uk>,
 Christian Gmeiner <christian.gmeiner@...il.com>, Jani Nikula
 <jani.nikula@...ux.intel.com>, Joonas Lahtinen
 <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>,
 Tvrtko Ursulin <tursulin@...ulin.net>, Frank Binns
 <frank.binns@...tec.com>, Matt Coster <matt.coster@...tec.com>, Qiang Yu
 <yuq825@...il.com>, Rob Clark <robdclark@...il.com>, Sean Paul
 <sean@...rly.run>, Konrad Dybcio <konradybcio@...nel.org>, Abhinav Kumar
 <quic_abhinavk@...cinc.com>, Dmitry Baryshkov
 <dmitry.baryshkov@...aro.org>, Marijn Suijten
 <marijn.suijten@...ainline.org>, Lyude Paul <lyude@...hat.com>, Danilo
 Krummrich <dakr@...nel.org>, Rob Herring <robh@...nel.org>, Steven Price
 <steven.price@....com>, Dave Airlie <airlied@...hat.com>, Gerd Hoffmann
 <kraxel@...hat.com>, Matthew Brost <matthew.brost@...el.com>, Huang Rui
 <ray.huang@....com>, Matthew Auld <matthew.auld@...el.com>, Melissa Wen
 <mwen@...lia.com>, Maíra Canal <mcanal@...lia.com>, Zack
 Rusin <zack.rusin@...adcom.com>, Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>, Lucas De Marchi
 <lucas.demarchi@...el.com>, Thomas Hellström
 <thomas.hellstrom@...ux.intel.com>, Bas Nieuwenhuizen
 <bas@...nieuwenhuizen.nl>, Yang Wang <kevinyang.wang@....com>, Jesse Zhang
 <jesse.zhang@....com>, Tim Huang <tim.huang@....com>, Sathishkumar S
 <sathishkumar.sundararaju@....com>, Saleemkhan Jamadar
 <saleemkhan.jamadar@....com>, Sunil Khatri <sunil.khatri@....com>, Lijo
 Lazar <lijo.lazar@....com>, Hawking Zhang <Hawking.Zhang@....com>, Ma Jun
 <Jun.Ma2@....com>, Yunxiang Li <Yunxiang.Li@....com>, Eric Huang
 <jinhuieric.huang@....com>, Asad Kamal <asad.kamal@....com>, Srinivasan
 Shanmugam <srinivasan.shanmugam@....com>, Jack Xiao <Jack.Xiao@....com>,
 Friedrich Vock <friedrich.vock@....de>, Michel Dänzer
 <mdaenzer@...hat.com>, Geert Uytterhoeven <geert@...ux-m68k.org>,
 Anna-Maria Behnsen <anna-maria@...utronix.de>, Thomas Gleixner
 <tglx@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>, Dan
 Carpenter <dan.carpenter@...aro.org>, linux-media@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
 linux-kernel@...r.kernel.org, amd-gfx@...ts.freedesktop.org,
 etnaviv@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
 lima@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
 freedreno@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
 virtualization@...ts.linux.dev, spice-devel@...ts.freedesktop.org,
 intel-xe@...ts.freedesktop.org
Subject: Re: [PATCH 1/2] dma-fence: Rename dma_fence_is_signaled()

Hi Philipp,

On Wed,  9 Apr 2025 14:06:37 +0200
Philipp Stanner <phasta@...nel.org> wrote:

> 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.
> 
> 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.
> 
> (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.
> 
> Give the function a more obvious name.

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.

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.

Regards,

Boris

Powered by blists - more mailing lists