[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aSctt3QFiEIB61Gr@lstrano-desk.jf.intel.com>
Date: Wed, 26 Nov 2025 08:41:27 -0800
From: Matthew Brost <matthew.brost@...el.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>, David Airlie <airlied@...il.com>,
"Simona Vetter" <simona@...ll.ch>, 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>, Huang Rui
<ray.huang@....com>, Matthew Auld <matthew.auld@...el.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Lucas De Marchi
<lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, <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>,
<intel-gfx@...ts.freedesktop.org>, <intel-xe@...ts.freedesktop.org>,
<rust-for-linux@...r.kernel.org>, Tvrtko Ursulin <tvrtko.ursulin@...lia.com>
Subject: Re: [PATCH 1/6] dma-buf/dma-fence: Add dma_fence_test_signaled_flag()
On Wed, Nov 26, 2025 at 02:19:10PM +0100, Philipp Stanner wrote:
> The dma_fence framework checks at many places whether the signaled flag
> of a fence is already set. The code can be simplified and made more
> readable by providing a helper function for that.
>
> Add dma_fence_test_signaled_flag(), which only checks whether a fence is
> signaled. Use it internally.
>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@...lia.com>
> Signed-off-by: Philipp Stanner <phasta@...nel.org>
This is a nice cleanp:
Reviewed-by: Matthew Brost <matthew.brost@...el.com>
> ---
> drivers/dma-buf/dma-fence.c | 19 +++++++++----------
> include/linux/dma-fence.h | 24 ++++++++++++++++++++++--
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 39e6f93dc310..25117a906846 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -372,8 +372,7 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>
> lockdep_assert_held(fence->lock);
>
> - if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> - &fence->flags)))
> + if (unlikely(dma_fence_test_signaled_flag(fence)))
> return -EINVAL;
>
> /* Stash the cb_list before replacing it with the timestamp */
> @@ -545,7 +544,7 @@ void dma_fence_release(struct kref *kref)
> trace_dma_fence_destroy(fence);
>
> if (!list_empty(&fence->cb_list) &&
> - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> + !dma_fence_test_signaled_flag(fence)) {
> const char __rcu *timeline;
> const char __rcu *driver;
> unsigned long flags;
> @@ -602,7 +601,7 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
> was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> &fence->flags);
>
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (dma_fence_test_signaled_flag(fence))
> return false;
>
> if (!was_set && fence->ops->enable_signaling) {
> @@ -666,7 +665,7 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
> if (WARN_ON(!fence || !func))
> return -EINVAL;
>
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> + if (dma_fence_test_signaled_flag(fence)) {
> INIT_LIST_HEAD(&cb->node);
> return -ENOENT;
> }
> @@ -783,7 +782,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>
> spin_lock_irqsave(fence->lock, flags);
>
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (dma_fence_test_signaled_flag(fence))
> goto out;
>
> if (intr && signal_pending(current)) {
> @@ -800,7 +799,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
> cb.task = current;
> list_add(&cb.base.node, &fence->cb_list);
>
> - while (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && ret > 0) {
> + while (!dma_fence_test_signaled_flag(fence) && ret > 0) {
> if (intr)
> __set_current_state(TASK_INTERRUPTIBLE);
> else
> @@ -832,7 +831,7 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>
> for (i = 0; i < count; ++i) {
> struct dma_fence *fence = fences[i];
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> + if (dma_fence_test_signaled_flag(fence)) {
> if (idx)
> *idx = i;
> return true;
> @@ -1108,7 +1107,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
> RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
> "RCU protection is required for safe access to returned string");
>
> - if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (!dma_fence_test_signaled_flag(fence))
> return fence->ops->get_driver_name(fence);
> else
> return "detached-driver";
> @@ -1140,7 +1139,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence)
> RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
> "RCU protection is required for safe access to returned string");
>
> - if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (!dma_fence_test_signaled_flag(fence))
> return fence->ops->get_timeline_name(fence);
> else
> return "signaled-timeline";
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 64639e104110..19972f5d176f 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -401,6 +401,26 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
> const char __rcu *dma_fence_driver_name(struct dma_fence *fence);
> const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
>
> +/*
> + * dma_fence_test_signaled_flag - Only check whether a fence is signaled yet.
> + * @fence: the fence to check
> + *
> + * This function just checks whether @fence is signaled, without interacting
> + * with the fence in any way. The user must, therefore, ensure through other
> + * means that fences get signaled eventually.
> + *
> + * This function uses test_bit(), which is thread-safe. Naturally, this function
> + * should be used opportunistically; a fence could get signaled at any moment
> + * after the check is done.
> + *
> + * Return: true if signaled, false otherwise.
> + */
> +static inline bool
> +dma_fence_test_signaled_flag(struct dma_fence *fence)
> +{
> + return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +}
> +
> /**
> * dma_fence_is_signaled_locked - Return an indication if the fence
> * is signaled yet.
> @@ -418,7 +438,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
> static inline bool
> dma_fence_is_signaled_locked(struct dma_fence *fence)
> {
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (dma_fence_test_signaled_flag(fence))
> return true;
>
> if (fence->ops->signaled && fence->ops->signaled(fence)) {
> @@ -448,7 +468,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
> static inline bool
> dma_fence_is_signaled(struct dma_fence *fence)
> {
> - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + if (dma_fence_test_signaled_flag(fence))
> return true;
>
> if (fence->ops->signaled && fence->ops->signaled(fence)) {
> --
> 2.49.0
>
Powered by blists - more mailing lists