[PATCH 0/2] dma-fence: Order is-signaled-bit

[PATCH 0/2] dma-fence: Order is-signaled-bit

[Philipp Stanner]

Changes in v2:
  - Add a set_bit_release() function for proper ordering and use it.

Based on the DRM branch drm-misc-next

Philipp Stanner (2):
  asm-generic/bitops: Add set_bit_release()
  dma-buf/dma-fence: Order setting of the signaled-bit properly

 drivers/dma-buf/dma-fence.c                      | 11 +++++++++--
 include/asm-generic/bitops/instrumented-atomic.h | 14 ++++++++++++++
 include/linux/dma-fence.h                        |  6 +++++-
 3 files changed, 28 insertions(+), 3 deletions(-)


base-commit: 1ecab8b0bad393750c60cc3819ce9c256595463c
-- 
2.56.0

[PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Philipp Stanner]

The kernel already has test_bit_acquire(). A set_bit_release(), however,
was missing so far. It can be expected to be useful for parties that
need release ordering, but not the stricter full ordering of the other
bit-setting functions.

Add set_bit_release().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 include/asm-generic/bitops/instrumented-atomic.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/asm-generic/bitops/instrumented-atomic.h b/include/asm-generic/bitops/instrumented-atomic.h
index c010d54275e4..fb2d1472fd4d 100644
--- a/include/asm-generic/bitops/instrumented-atomic.h
+++ b/include/asm-generic/bitops/instrumented-atomic.h
@@ -72,6 +72,20 @@ static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *ad
 	return arch_test_and_set_bit(nr, addr);
 }
 
+/**
+ * set_bit_release - Set a bit with release ordering
+ * @nr: Bit to set
+ * @addr: Address to start counting from
+ *
+ * This is an atomic ordered operation (implied write memory barrier).
+ */
+static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
+{
+	kcsan_wmb();
+	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
+	arch_set_bit(nr, addr);
+}
+
 /**
  * test_and_clear_bit - Clear a bit and return its old value
  * @nr: Bit to clear
-- 
2.56.0

[PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[Philipp Stanner]

dma_fence_is_signaled() can return true *while* the fence is still
signaling. This is fragile and has been involved in at least two bugs:

	commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
	commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")

The signaled-bit is set under lock-protection but read without it.
Adding the lock has been rejected on the grounds of the lock-acquisition
shooting down cache lines of parties that merely want to check whether
the fence is already signaled.

Make sure the bit can only become visible as 1 once the callbacks have
ran. To achieve this, read the bit with test_bit_acquire() and set it
with set_bit_release().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/dma-fence.c | 11 +++++++++--
 include/linux/dma-fence.h   |  6 +++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index bd58688b81a7..07d499fa35ac 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -359,8 +359,7 @@ 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)))
+	if (unlikely(dma_fence_test_signaled_flag(fence)))
 		return;
 
 	trace_dma_fence_signaled(fence);
@@ -384,6 +383,14 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 		INIT_LIST_HEAD(&cur->node);
 		cur->func(fence, cur);
 	}
+
+	/*
+	 * This orders the signaled-state with the test_bit_acquire() in
+	 * dma_fence_test_signaled_flag(). This ensures that functions like
+	 * dma_fence_is_signaled() can only ever return true once signaling is
+	 * completed (i.e., after the callbacks above ran).
+	 */
+	set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
 }
 EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
 
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index dd07d128adc3..61522027af41 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -496,7 +496,11 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
 static inline bool
 dma_fence_test_signaled_flag(struct dma_fence *fence)
 {
-	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
+	/*
+	 * The counterpart of this ordered operation can be found in
+	 * dma_fence_signal_timestamp_locked().
+	 */
+	return test_bit_acquire(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
 }
 
 /**
-- 
2.56.0

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bitops: set_bit_release() lacks release barrier and uses kcsan_wmb()
- [Medium] bitops: set_bit_release() undefined on archs without instrumented-atomic.h
--

commit e20f77caba7042f428258dbb5808e1ad22ab5a60
Author: Philipp Stanner <phasta@kernel.org>

asm-generic/bitops: Add set_bit_release()

This commit adds set_bit_release() to
include/asm-generic/bitops/instrumented-atomic.h to provide a bit-setting
primitive with release ordering that pairs with test_bit_acquire().

> diff --git a/include/asm-generic/bitops/instrumented-atomic.h b/include/asm-generic/bitops/instrumented-atomic.h
> index c010d54275e4a..fb2d1472fd4d5 100644
> --- a/include/asm-generic/bitops/instrumented-atomic.h
> +++ b/include/asm-generic/bitops/instrumented-atomic.h
> @@ -72,6 +72,20 @@ static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *ad
>  	return arch_test_and_set_bit(nr, addr);
>  }
>  
> +/**
> + * set_bit_release - Set a bit with release ordering
> + * @nr: Bit to set
> + * @addr: Address to start counting from
> + *
> + * This is an atomic ordered operation (implied write memory barrier).
> + */
> +static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)

[Severity: Medium]
Will defining set_bit_release() only in
include/asm-generic/bitops/instrumented-atomic.h break the build when generic
kernel code calls set_bit_release() on architectures that do not include this
header?

Several architectures define their own atomic bitops directly in
arch/*/include/asm/bitops.h without including
include/asm-generic/bitops/instrumented-atomic.h or
include/asm-generic/bitops/atomic.h, including 32-bit arm, mips, alpha, m68k,
parisc, sparc, hexagon, openrisc, and sh:

arch/arm/include/asm/bitops.h:
    #define set_bit(nr,p)			ATOMIC_BITOP(set_bit,nr,p)
    #define clear_bit(nr,p)			ATOMIC_BITOP(clear_bit,nr,p)
    #define change_bit(nr,p)		ATOMIC_BITOP(change_bit,nr,p)

arch/mips/include/asm/bitops.h:
    static inline void set_bit(unsigned long nr, volatile unsigned long *addr)

Unlike clear_bit_unlock() (provided via include/asm-generic/bitops/lock.h and
architecture-specific definitions) or test_bit_acquire() (wired through
include/linux/bitops.h and include/asm-generic/bitops/non-atomic.h),
set_bit_release() has no definition or fallback on those architectures.

> +{
> +	kcsan_wmb();
> +	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
> +	arch_set_bit(nr, addr);
> +}

[Severity: Critical]
Does set_bit_release() in include/asm-generic/bitops/instrumented-atomic.h
actually provide release memory ordering when a caller publishes state to a
concurrent reader using test_bit_acquire()?

First, arch_set_bit() is a relaxed (unordered) atomic operation. In
include/asm-generic/bitops/atomic.h (used by arm64, loongarch, arc, csky,
s390, and others), arch_set_bit() calls raw_atomic_long_or(), which is
unordered and has no release barrier:

include/asm-generic/bitops/atomic.h:
    static __always_inline void
    arch_set_bit(unsigned int nr, volatile unsigned long *p)
    {
    	p += BIT_WORD(nr);
    	raw_atomic_long_or(BIT_MASK(nr), (atomic_long_t *)p);
    }

Architecture-specific implementations of arch_set_bit() in
arch/powerpc/include/asm/bitops.h and arch/riscv/include/asm/bitops.h are
likewise relaxed. By contrast, release-ordered bitops in
include/asm-generic/bitops/lock.h use a release-ordered atomic operation:

include/asm-generic/bitops/lock.h:
    static __always_inline void
    arch_clear_bit_unlock(unsigned int nr, volatile unsigned long *addr)
    {
    	addr += BIT_WORD(nr);
    	raw_atomic_long_fetch_andnot_release(BIT_MASK(nr), (atomic_long_t *)addr);
    }

Second, kcsan_wmb() in include/linux/kcsan-checks.h is only a sanitizer
annotation that compiles to a no-op when weak memory modeling is disabled and
never emits a hardware memory barrier:

include/linux/kcsan-checks.h:
    #define kcsan_wmb()	do { } while (0)
    #define kcsan_rmb()	do { } while (0)
    #define kcsan_release()	do { } while (0)

Third, both kcsan_wmb() and the function comment ("implied write memory
barrier") model a write memory barrier rather than a release operation. In
kernel/kcsan/core.c, wmb only orders prior writes, whereas release orders both
prior reads and prior writes:

kernel/kcsan/core.c:
    DEFINE_MEMORY_BARRIER(mb, true);
    DEFINE_MEMORY_BARRIER(wmb, sa->type & (KCSAN_ACCESS_WRITE | KCSAN_ACCESS_COMPOUND));
    DEFINE_MEMORY_BARRIER(rmb, !(sa->type & KCSAN_ACCESS_WRITE) || (sa->type & KCSAN_ACCESS_COMPOUND));
    DEFINE_MEMORY_BARRIER(release, true);

Because there is no hardware or compiler release barrier here, can the
compiler and processor on weakly-ordered architectures reorder prior memory
reads and writes after arch_set_bit(), causing data races when paired with
test_bit_acquire()? Should this follow clear_bit_unlock() in
include/asm-generic/bitops/instrumented-lock.h by using kcsan_release() and a
release-ordered arch_set_bit_release() helper?

include/asm-generic/bitops/instrumented-lock.h:
    static inline void clear_bit_unlock(long nr, volatile unsigned long *addr)
    {
    	kcsan_release();
    	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
    	arch_clear_bit_unlock(nr, addr);
    }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021095512.1345191-2-phasta@kernel.org?part=1

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] dma-fence: deadlock and UAF in dma_fence_release() on put in callback
- [Critical] dma-fence: broken state and re-entrancy in dma_fence_signal_timestamp_locked()
- [High] dma-fence: lost wakeups in waiters when running callbacks before setting bit
- [High] dma-fence: build failure and missing barrier from set_bit_release()
--

commit 1792aa61ea7d70257b20df7aab8e20f6da404551
Author: Philipp Stanner <phasta@kernel.org>

dma-buf/dma-fence: Order setting of the signaled-bit properly

This commit defers setting DMA_FENCE_FLAG_SIGNALED_BIT in
dma_fence_signal_timestamp_locked() until after all fence callbacks have
completed, using set_bit_release() and test_bit_acquire() to order the
signaled state.

> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index bd58688b81a72..07d499fa35ac9 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -359,8 +359,7 @@ 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)))
> +	if (unlikely(dma_fence_test_signaled_flag(fence)))
>  		return;

[Severity: Critical]
Previously, test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)
marked the fence as signaled at entry before clearing fence->ops, replacing
fence->cb_list with fence->timestamp, and invoking callbacks. With
DMA_FENCE_FLAG_SIGNALED_BIT remaining zero until the end of
dma_fence_signal_timestamp_locked(), what happens if a callback queries or
signals the fence?

In include/linux/dma-fence.h:

    static inline bool
    dma_fence_is_signaled_locked(struct dma_fence *fence)
    {
            const struct dma_fence_ops *ops;

            if (dma_fence_test_signaled_flag(fence))
                    return true;

            rcu_read_lock();
            ops = rcu_dereference(fence->ops);
            if (ops && ops->signaled && ops->signaled(fence)) {
                    rcu_read_unlock();
                    dma_fence_signal_locked(fence);
                    return true;
            }
            rcu_read_unlock();

            return false;
    }

If a callback calls dma_fence_signal_locked(fence), or calls
dma_fence_is_signaled_locked(fence) or dma_fence_get_status_locked(fence)
when ops->signaled(fence) returns true, dma_fence_signal_timestamp_locked()
is re-entered while DMA_FENCE_FLAG_SIGNALED_BIT is still zero. If
fence->ops was cleared to NULL (when !ops->release && !ops->wait), the
recursive call dereferences a NULL ops pointer; otherwise,
list_replace(&fence->cb_list, &cb_list) runs on the union already
overwritten by fence->timestamp, corrupting memory by treating the timestamp
as list pointers.

Conversely, if ops->signaled is NULL (or fence->ops was cleared), calling
dma_fence_is_signaled_locked(fence) or dma_fence_get_status_locked(fence)
from a callback now returns false or 0 for a fence that is being signaled.
If a callback instead calls unlocked helpers such as
dma_fence_is_signaled(fence) (when ops->signaled returns true) or
dma_fence_add_callback(fence, ...), dma_fence_test_signaled_flag(fence)
returns false and those helpers attempt to re-acquire fence->lock on the
same CPU, causing a spinlock self-deadlock.

Similarly, in include/linux/dma-fence.h:

    static inline ktime_t dma_fence_timestamp(struct dma_fence *fence)
    {
            if (WARN_ON(!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)))
                    return ktime_get();

            while (!test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags))
                    cpu_relax();

            return fence->timestamp;
    }

Won't calling dma_fence_timestamp(fence) from a fence callback (or from a
worker woken by a callback before set_bit_release() runs) now trigger the
WARN_ON and return ktime_get() even though fence->timestamp and
DMA_FENCE_FLAG_TIMESTAMP_BIT have already been set?

>  
>  	trace_dma_fence_signaled(fence);
> @@ -384,6 +383,14 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  		INIT_LIST_HEAD(&cur->node);
>  		cur->func(fence, cur);
>  	}

[Severity: Critical]
In struct dma_fence, cb_list, timestamp, and rcu share a union whose
comment notes that cb_list is only valid until the signal bit is set:

    union {
            struct list_head cb_list;
            /* @cb_list replaced by @timestamp on dma_fence_signal() */
            ktime_t timestamp;
            /* @timestamp replaced by @rcu on dma_fence_release() */
            struct rcu_head rcu;
    };

In dma_fence_signal_timestamp_locked(), list_replace(&fence->cb_list,
&cb_list) and fence->timestamp = timestamp overwrite that union before
cur->func(fence, cur) runs, while DMA_FENCE_FLAG_SIGNALED_BIT is still 0.

Many in-tree callbacks drop a reference to the fence via dma_fence_put()
inside cur->func(), such as dma_fence_chain_cb(), dma_buf_poll_cb(),
drm_sched_entity_wakeup(), drm_sched_entity_kill_jobs_cb(),
vmw_event_fence_action_seq_passed(), amdgpu_pasid_free_cb(), and
amdgpu_display_flip_callback(). When the signaler does not hold an extra
reference across dma_fence_signal_locked() (for example in
sw_sync_debugfs_release() or __vmw_fences_update()), or when a waiter woken
by cur->func() drops the last reference before set_bit_release() executes,
dma_fence_put() invokes dma_fence_release():

    if (!list_empty(&fence->cb_list) &&
        !dma_fence_test_signaled_flag(fence)) {
            ...
            WARN(1,
                 "Fence %s:%s:%llx:%llx released with pending signals!\n",
                 ...);

            dma_fence_lock_irqsave(fence, flags);
            fence->error = -EDEADLK;
            dma_fence_signal_locked(fence);
            dma_fence_unlock_irqrestore(fence, flags);
    }

Because fence->cb_list now holds fence->timestamp (making !list_empty()
evaluate to true) and DMA_FENCE_FLAG_SIGNALED_BIT is not yet set, won't
dma_fence_release() emit a false WARN, self-deadlock on fence->lock when
called from cur->func() (or re-enter dma_fence_signal_locked() on a
corrupted cb_list from another CPU), and then release or free the fence
before dma_fence_signal_timestamp_locked() writes to fence->flags and
unlocks fence->inline_lock?

> +
> +	/*
> +	 * This orders the signaled-state with the test_bit_acquire() in
> +	 * dma_fence_test_signaled_flag(). This ensures that functions like
> +	 * dma_fence_is_signaled() can only ever return true once signaling is
> +	 * completed (i.e., after the callbacks above ran).
> +	 */

[Severity: High]
Could deferring DMA_FENCE_FLAG_SIGNALED_BIT until after the callbacks have
run cause lost wakeups for waiters, pollers, and work items that are woken
by cur->func()?

Waiters register a one-shot callback with dma_fence_add_callback() that
wakes the task or waitqueue (for example dma_fence_default_wait_cb(),
syncobj_wait_fence_func(), fence_check_cb_func(), and
radeon_fence_wait_cb()), and then check whether the fence is signaled
without acquiring fence->lock.

For instance, in drivers/dma-buf/dma-fence.c:dma_fence_wait_any_timeout():

    while (ret > 0) {
            if (intr)
                    set_current_state(TASK_INTERRUPTIBLE);
            else
                    set_current_state(TASK_UNINTERRUPTIBLE);

            if (dma_fence_test_signaled_any(fences, count, idx))
                    break;

            ret = schedule_timeout(ret);

If CPU0 runs dma_fence_default_wait_cb() and wakes the waiting task on CPU1
before CPU0 finishes the callback loop and calls set_bit_release(), CPU1 can
set its task state and call dma_fence_test_signaled_any() ->
dma_fence_test_signaled_flag(fence), observe DMA_FENCE_FLAG_SIGNALED_BIT ==
0, and go to sleep in schedule_timeout(ret) with its callback already
removed and executed.

Doesn't the same race also affect:

1. drm_syncobj_array_wait_timeout() in drivers/gpu/drm/drm_syncobj.c, where
   woken task checks dma_fence_is_signaled(fence) while
   entries[i].fence_cb.func is already non-NULL (and after fence->ops was
   cleared to NULL), sees false, and sleeps again in schedule_timeout()?

2. sync_file_poll() in drivers/dma-buf/sync_file.c, where
   fence_check_cb_func() wakes sync_file->wq while
   DMA_FENCE_FLAG_SIGNALED_BIT is still 0, causing the woken poller to see
   POLL_ENABLED already set and dma_fence_is_signaled(sync_file->fence)
   returning false, missing EPOLLIN?

3. radeon_fence_default_wait() in drivers/gpu/drm/radeon/radeon_fence.c and
   __i915_request_wait() in drivers/gpu/drm/i915/i915_request.c, which wake
   the waiter from a fence callback and test DMA_FENCE_FLAG_SIGNALED_BIT
   before calling schedule_timeout()?

4. nouveau_cli_work() in drivers/gpu/drm/nouveau/nouveau_drm.c, where
   nouveau_cli_work_fence() schedules cli->work from the callback, and if
   nouveau_cli_work() runs before set_bit_release() and sees
   dma_fence_is_signaled(fence) == false, it skips the work item with no
   further callback to reschedule it?

> +	set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);

[Severity: High]
Does set_bit_release() build on all architectures and provide a hardware
release barrier on weakly-ordered CPUs?

set_bit_release() is only defined in
include/asm-generic/bitops/instrumented-atomic.h:

    static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
    {
            kcsan_wmb();
            instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
            arch_set_bit(nr, addr);
    }

Architectures that define their own atomic bitops without including
asm-generic/bitops/instrumented-atomic.h (such as 32-bit arm, mips, sparc,
parisc, m68k, alpha, sh, openrisc, and hexagon) do not declare
set_bit_release(), which breaks compiling drivers/dma-buf/dma-fence.c.

Furthermore, on architectures that do include
asm-generic/bitops/instrumented-atomic.h (such as arm64, riscv, powerpc, and
loongarch), kcsan_wmb() is only a KCSAN annotation (a no-op when
CONFIG_KCSAN=n) and arch_set_bit() is a relaxed atomic operation without
release semantics:

    static __always_inline void
    arch_set_bit(unsigned int nr, volatile unsigned long *p)
    {
            p += BIT_WORD(nr);
            raw_atomic_long_or(BIT_MASK(nr), (atomic_long_t *)p);
    }

Unlike test_and_set_bit(), which implies a full memory barrier, doesn't
set_bit_release() omit the CPU release barrier needed to pair with
test_bit_acquire() on weakly-ordered architectures?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021095512.1345191-2-phasta@kernel.org?part=2

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Arnd Bergmann]

On Wed, Oct 21, 2026, at 11:55, Philipp Stanner wrote:
> The kernel already has test_bit_acquire(). A set_bit_release(), however,
> was missing so far. It can be expected to be useful for parties that
> need release ordering, but not the stricter full ordering of the other
> bit-setting functions.
>
> Add set_bit_release().
>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

> +/**
> + * set_bit_release - Set a bit with release ordering
> + * @nr: Bit to set
> + * @addr: Address to start counting from
> + *
> + * This is an atomic ordered operation (implied write memory barrier).
> + */

Can we fix the mismatch between code comments and documentation for
the set_bit()/clear_bit()/change_bit() barrier semantics while adding
this? I'm always confused by this and the new function makes it harder
to understand in the current form:

- Your patch description mentions that set_bit() has strong rw
  barrier semantics and that set_bit_release() would be a weaker
  write-barrier version of it. This makes sense given the
  set_bit() implementation based on strong raw_atomic_long_or()
  in arch_set_bit() and the text in Documentation/memory-barriers.txt

- Contrary to that, the kerneldoc comment over set_bit() mentions
  "This is a relaxed atomic operation (no implied memory barriers)"
  and leaves out the kcsan_mb() that is in test_and_set_bit().
  This makes the new set_bit_release() sound like a /stronger/
  version of set_bit() the same way that test_bit_acquire()
  is a stronger version of the (relaxed) test_bit().

As I understand it, the kerneldoc comments here are incorrect
and trace back to the include/asm-i386/bitops.h file before the
git era. The patch below should make them consistent with your
patch and with the actual implementation.

       Arnd

diff --git a/include/asm-generic/bitops/instrumented-atomic.h b/include/asm-generic/bitops/instrumented-atomic.h
index c010d54275e4..df200802e929 100644
--- a/include/asm-generic/bitops/instrumented-atomic.h
+++ b/include/asm-generic/bitops/instrumented-atomic.h
@@ -18,13 +18,14 @@
  * @nr: the bit to set
  * @addr: the address to start counting from
  *
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
  *
  * Note that @nr may be almost arbitrarily large; this function is not
  * restricted to acting on a single-word quantity.
  */
 static __always_inline void set_bit(long nr, volatile unsigned long *addr)
 {
+	kcsan_mb();
 	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
 	arch_set_bit(nr, addr);
 }
@@ -34,10 +35,11 @@ static __always_inline void set_bit(long nr, volatile unsigned long *addr)
  * @nr: Bit to clear
  * @addr: Address to start counting from
  *
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
  */
 static __always_inline void clear_bit(long nr, volatile unsigned long *addr)
 {
+	kcsan_mb();
 	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
 	arch_clear_bit(nr, addr);
 }
@@ -47,13 +49,14 @@ static __always_inline void clear_bit(long nr, volatile unsigned long *addr)
  * @nr: Bit to change
  * @addr: Address to start counting from
  *
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
  *
  * Note that @nr may be almost arbitrarily large; this function is not
  * restricted to acting on a single-word quantity.
  */
 static __always_inline void change_bit(long nr, volatile unsigned long *addr)
 {
+	kcsan_mb();
 	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
 	arch_change_bit(nr, addr);
 }

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Gary Guo]

On Wed Oct 21, 2026 at 12:33 PM BST, Arnd Bergmann wrote:
> On Wed, Oct 21, 2026, at 11:55, Philipp Stanner wrote:
>> The kernel already has test_bit_acquire(). A set_bit_release(), however,
>> was missing so far. It can be expected to be useful for parties that
>> need release ordering, but not the stricter full ordering of the other
>> bit-setting functions.
>>
>> Add set_bit_release().
>>
>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>
>> +/**
>> + * set_bit_release - Set a bit with release ordering
>> + * @nr: Bit to set
>> + * @addr: Address to start counting from
>> + *
>> + * This is an atomic ordered operation (implied write memory barrier).
>> + */
>
> Can we fix the mismatch between code comments and documentation for
> the set_bit()/clear_bit()/change_bit() barrier semantics while adding
> this? I'm always confused by this and the new function makes it harder
> to understand in the current form:
>
> - Your patch description mentions that set_bit() has strong rw
>   barrier semantics and that set_bit_release() would be a weaker
>   write-barrier version of it. This makes sense given the
>   set_bit() implementation based on strong raw_atomic_long_or()
>   in arch_set_bit() and the text in Documentation/memory-barriers.txt
>
> - Contrary to that, the kerneldoc comment over set_bit() mentions
>   "This is a relaxed atomic operation (no implied memory barriers)"
>   and leaves out the kcsan_mb() that is in test_and_set_bit().
>   This makes the new set_bit_release() sound like a /stronger/
>   version of set_bit() the same way that test_bit_acquire()
>   is a stronger version of the (relaxed) test_bit().

set_bit has relaxed semantics while test_and_set_bit implies barrier. RISC-V for
exmaple will generate the amoor instruction for set_bit, not amoor.aqrl.

Best,
Gary

>
> As I understand it, the kerneldoc comments here are incorrect
> and trace back to the include/asm-i386/bitops.h file before the
> git era. The patch below should make them consistent with your
> patch and with the actual implementation.
>
>        Arnd

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Gary Guo]

On Wed Oct 21, 2026 at 10:55 AM BST, Philipp Stanner wrote:
> The kernel already has test_bit_acquire(). A set_bit_release(), however,
> was missing so far. It can be expected to be useful for parties that
> need release ordering, but not the stricter full ordering of the other
> bit-setting functions.
>
> Add set_bit_release().
>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  include/asm-generic/bitops/instrumented-atomic.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/asm-generic/bitops/instrumented-atomic.h b/include/asm-generic/bitops/instrumented-atomic.h
> index c010d54275e4..fb2d1472fd4d 100644
> --- a/include/asm-generic/bitops/instrumented-atomic.h
> +++ b/include/asm-generic/bitops/instrumented-atomic.h
> @@ -72,6 +72,20 @@ static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *ad
>  	return arch_test_and_set_bit(nr, addr);
>  }
>  
> +/**
> + * set_bit_release - Set a bit with release ordering
> + * @nr: Bit to set
> + * @addr: Address to start counting from
> + *
> + * This is an atomic ordered operation (implied write memory barrier).
> + */
> +static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
> +{
> +	kcsan_wmb();
> +	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
> +	arch_set_bit(nr, addr);

This is just wrong. There's no code here that gives you the release semantics.

kcsan_wmb() is just an annotation and does nothing. wmb is also not release
semantics as it does not order LOAD->STORE.

Given that set_bit is a relaxed RMW atomic, you can implement set_bit_release
with just

    smp_mb__before_atomic();
    set_bit(nr, addr);

Which should be correct, but is too strong on some architectures (like RISC-V),
becuase it will produce a full MB instead. So we can either add release barriers
(hint: I really like it), or we can add arch_set_bit_release and implement it
per arch; on RISC-V for example it can then be translated to amoor.rl (which is
not being produced by any existing bit ops).c

Best,
Gary

> +}
> +
>  /**
>   * test_and_clear_bit - Clear a bit and return its old value
>   * @nr: Bit to clear

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Arnd Bergmann]

On Wed, Oct 21, 2026, at 13:54, Gary Guo wrote:
> On Wed Oct 21, 2026 at 12:33 PM BST, Arnd Bergmann wrote:

>> I'm always confused by this and the new function makes it harder
>> to understand in the current form:
...
>> - Contrary to that, the kerneldoc comment over set_bit() mentions
>>   "This is a relaxed atomic operation (no implied memory barriers)"
>>   and leaves out the kcsan_mb() that is in test_and_set_bit().
>>   This makes the new set_bit_release() sound like a /stronger/
>>   version of set_bit() the same way that test_bit_acquire()
>>   is a stronger version of the (relaxed) test_bit().
>
> set_bit has relaxed semantics while test_and_set_bit implies barrier. RISC-V for
> exmaple will generate the amoor instruction for set_bit, not amoor.aqrl.

Thanks for the clarification, I did misread the implementation
and see how I was confused once more.

     Arnd

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Philipp Stanner]

On Wed, 2026-10-21 at 13:06 +0100, Gary Guo wrote:


[…]

> > +/**
> > + * set_bit_release - Set a bit with release ordering
> > + * @nr: Bit to set
> > + * @addr: Address to start counting from
> > + *
> > + * This is an atomic ordered operation (implied write memory barrier).
> > + */
> > +static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
> > +{
> > +	kcsan_wmb();
> > +	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
> > +	arch_set_bit(nr, addr);
> 
> This is just wrong. There's no code here that gives you the release semantics.
> 
> kcsan_wmb() is just an annotation and does nothing. wmb is also not release
> semantics as it does not order LOAD->STORE.

What are the other functions here then even about?

/**
 * test_and_set_bit - Set a bit and return its old value
 * @nr: Bit to set
 * @addr: Address to count from
 *
 * This is an atomic fully-ordered operation (implied full memory barrier).
 */
static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
{
	kcsan_mb();
	instrument_atomic_read_write(addr + BIT_WORD(nr), sizeof(long));
	return arch_test_and_set_bit(nr, addr);
}

Where does the "full ordering" come from? I suppose implied barrier
then means it comes from the architecture's implementation. But what's
then the purpose of kcsan_XX ?

> Given that set_bit is a relaxed RMW atomic, you can implement set_bit_release
> with just
> 
>     smp_mb__before_atomic();
>     set_bit(nr, addr);
> 
> Which should be correct, but is too strong on some architectures (like RISC-V),
> becuase it will produce a full MB instead. So we can either add release barriers
> (hint: I really like it), or we can add arch_set_bit_release and implement it
> per arch; on RISC-V for example it can then be translated to amoor.rl (which is
> not being produced by any existing bit ops).c


All fine by me.


P.

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Gary Guo]

On Wed Oct 21, 2026 at 1:17 PM BST, Philipp Stanner wrote:
> On Wed, 2026-10-21 at 13:06 +0100, Gary Guo wrote:
>
>
> […]
>
>> > +/**
>> > + * set_bit_release - Set a bit with release ordering
>> > + * @nr: Bit to set
>> > + * @addr: Address to start counting from
>> > + *
>> > + * This is an atomic ordered operation (implied write memory barrier).
>> > + */
>> > +static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
>> > +{
>> > +	kcsan_wmb();
>> > +	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
>> > +	arch_set_bit(nr, addr);
>> 
>> This is just wrong. There's no code here that gives you the release semantics.
>> 
>> kcsan_wmb() is just an annotation and does nothing. wmb is also not release
>> semantics as it does not order LOAD->STORE.
>
> What are the other functions here then even about?
>
> /**
>  * test_and_set_bit - Set a bit and return its old value
>  * @nr: Bit to set
>  * @addr: Address to count from
>  *
>  * This is an atomic fully-ordered operation (implied full memory barrier).
>  */
> static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
> {
> 	kcsan_mb();
> 	instrument_atomic_read_write(addr + BIT_WORD(nr), sizeof(long));
> 	return arch_test_and_set_bit(nr, addr);
> }
>
> Where does the "full ordering" come from? I suppose implied barrier
> then means it comes from the architecture's implementation. But what's
> then the purpose of kcsan_XX ?

It's from arch_test_and_set_bit.

KCSAN doesn't see arch code, so the kcsan_mb() and in fact
instrumnent_atomic_read_write() both are annotations to indicate that this is
conceptually a mb() + STORE for instrumentation purpose.

Best,
Gary

Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

[Philipp Stanner]

On Wed, 2026-10-21 at 13:24 +0100, Gary Guo wrote:
> On Wed Oct 21, 2026 at 1:17 PM BST, Philipp Stanner wrote:

[…]

> > static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
> > {
> > 	kcsan_mb();
> > 	instrument_atomic_read_write(addr + BIT_WORD(nr), sizeof(long));
> > 	return arch_test_and_set_bit(nr, addr);
> > }
> > 
> > Where does the "full ordering" come from? I suppose implied barrier
> > then means it comes from the architecture's implementation. But what's
> > then the purpose of kcsan_XX ?
> 
> It's from arch_test_and_set_bit.
> 
> KCSAN doesn't see arch code, so the kcsan_mb() and in fact
> instrumnent_atomic_read_write() both are annotations to indicate that this is
> conceptually a mb() + STORE for instrumentation purpose.


We (i.e., someone who's got the overview) could probably document where
and why our atomic instructions are a bit scattered.

Why is test_bit_acquire() defined in include/linux/bitops.h, and should
set_bit_release() be defined there – or here?


P.

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[Christian König]

On 10/21/26 11:55, Philipp Stanner wrote:
> dma_fence_is_signaled() can return true *while* the fence is still
> signaling. This is fragile and has been involved in at least two bugs:
> 
> 	commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
> 	commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")
> 
> The signaled-bit is set under lock-protection but read without it.
> Adding the lock has been rejected on the grounds of the lock-acquisition
> shooting down cache lines of parties that merely want to check whether
> the fence is already signaled.
> 
> Make sure the bit can only become visible as 1 once the callbacks have
> ran. To achieve this, read the bit with test_bit_acquire() and set it
> with set_bit_release().

That has been suggested and rejected before.

Background is that the signaling bit must be visible before the callbacks are run or things like dma_fence_default_wait() (as an example) just break.

Regards,
Christian.

> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/dma-buf/dma-fence.c | 11 +++++++++--
>  include/linux/dma-fence.h   |  6 +++++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index bd58688b81a7..07d499fa35ac 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -359,8 +359,7 @@ 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)))
> +	if (unlikely(dma_fence_test_signaled_flag(fence)))
>  		return;
>  
>  	trace_dma_fence_signaled(fence);
> @@ -384,6 +383,14 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  		INIT_LIST_HEAD(&cur->node);
>  		cur->func(fence, cur);
>  	}
> +
> +	/*
> +	 * This orders the signaled-state with the test_bit_acquire() in
> +	 * dma_fence_test_signaled_flag(). This ensures that functions like
> +	 * dma_fence_is_signaled() can only ever return true once signaling is
> +	 * completed (i.e., after the callbacks above ran).
> +	 */
> +	set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
>  }
>  EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>  
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index dd07d128adc3..61522027af41 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -496,7 +496,11 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
>  static inline bool
>  dma_fence_test_signaled_flag(struct dma_fence *fence)
>  {
> -	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +	/*
> +	 * The counterpart of this ordered operation can be found in
> +	 * dma_fence_signal_timestamp_locked().
> +	 */
> +	return test_bit_acquire(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
>  }
>  
>  /**

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[Philipp Stanner]

On Wed, 2026-10-21 at 15:47 +0200, Christian König wrote:
> On 10/21/26 11:55, Philipp Stanner wrote:
> > dma_fence_is_signaled() can return true *while* the fence is still
> > signaling. This is fragile and has been involved in at least two bugs:
> > 
> > 	commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
> > 	commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")
> > 
> > The signaled-bit is set under lock-protection but read without it.
> > Adding the lock has been rejected on the grounds of the lock-acquisition
> > shooting down cache lines of parties that merely want to check whether
> > the fence is already signaled.
> > 
> > Make sure the bit can only become visible as 1 once the callbacks have
> > ran. To achieve this, read the bit with test_bit_acquire() and set it
> > with set_bit_release().
> 
> That has been suggested and rejected before.
> 
> Background is that the signaling bit must be visible before the
> callbacks are run or things like dma_fence_default_wait() (as an
> example) just break.

dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
{
	struct default_wait_cb cb;
	unsigned long flags;
	signed long ret = timeout ? timeout : 1;

	dma_fence_lock_irqsave(fence, flags);

	if (dma_fence_test_signaled_flag(fence))
		goto out;

	if (intr && signal_pending(current)) {
		ret = -ERESTARTSYS;
		goto out;
	}

	if (!timeout) {
		ret = 0;
		goto out;
	}

	cb.base.func = dma_fence_default_wait_cb;
	cb.task = current;
	list_add(&cb.base.node, &fence->cb_list);

	while (!dma_fence_test_signaled_flag(fence) && ret > 0) {
		if (intr)
			__set_current_state(TASK_INTERRUPTIBLE);
		else
			__set_current_state(TASK_UNINTERRUPTIBLE);
		dma_fence_unlock_irqrestore(fence, flags);


That function evaluates the flag under lock-protection, with full
ordering. The bit is only ever set under lock-protection, with full
ordering.

So currently, the bit is 1 before the callbacks ran. It becomes 1
before the callbacks ran, but dma_fence_default_wait() can only see
that after it took the lock, and it can only do that after the lock is
released after the callback ran.

So behavior for this function is unchanged, unless I miss sth.

Actually, this barrier makes things more robust, since things like

if (dma_fence_is_signaled(f))
  ts = dma_fence_timestamp(f)

could misorder the accesses to the signaled-bit.


Do you have another example of what would break?


You see, the bit sometimes being locked and sometimes being lockless
causes confusion.


P.

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[Christian König]

On 10/21/26 16:52, Philipp Stanner wrote:
> On Wed, 2026-10-21 at 15:47 +0200, Christian König wrote:
>> On 10/21/26 11:55, Philipp Stanner wrote:
>>> dma_fence_is_signaled() can return true *while* the fence is still
>>> signaling. This is fragile and has been involved in at least two bugs:
>>>
>>> 	commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
>>> 	commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")
>>>
>>> The signaled-bit is set under lock-protection but read without it.
>>> Adding the lock has been rejected on the grounds of the lock-acquisition
>>> shooting down cache lines of parties that merely want to check whether
>>> the fence is already signaled.
>>>
>>> Make sure the bit can only become visible as 1 once the callbacks have
>>> ran. To achieve this, read the bit with test_bit_acquire() and set it
>>> with set_bit_release().
>>
>> That has been suggested and rejected before.
>>
>> Background is that the signaling bit must be visible before the
>> callbacks are run or things like dma_fence_default_wait() (as an
>> example) just break.
> 
> dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
> {
> 	struct default_wait_cb cb;
> 	unsigned long flags;
> 	signed long ret = timeout ? timeout : 1;
> 
> 	dma_fence_lock_irqsave(fence, flags);
> 
> 	if (dma_fence_test_signaled_flag(fence))
> 		goto out;
> 
> 	if (intr && signal_pending(current)) {
> 		ret = -ERESTARTSYS;
> 		goto out;
> 	}
> 
> 	if (!timeout) {
> 		ret = 0;
> 		goto out;
> 	}
> 
> 	cb.base.func = dma_fence_default_wait_cb;
> 	cb.task = current;
> 	list_add(&cb.base.node, &fence->cb_list);
> 
> 	while (!dma_fence_test_signaled_flag(fence) && ret > 0) {
> 		if (intr)
> 			__set_current_state(TASK_INTERRUPTIBLE);
> 		else
> 			__set_current_state(TASK_UNINTERRUPTIBLE);
> 		dma_fence_unlock_irqrestore(fence, flags);
> 
> 
> That function evaluates the flag under lock-protection, with full
> ordering. The bit is only ever set under lock-protection, with full
> ordering.
> 
> So currently, the bit is 1 before the callbacks ran. It becomes 1
> before the callbacks ran, but dma_fence_default_wait() can only see
> that after it took the lock, and it can only do that after the lock is
> released after the callback ran.
> 
> So behavior for this function is unchanged, unless I miss sth.

Yeah that argument came up before and no it is not correct.

> Actually, this barrier makes things more robust, since things like
> 
> if (dma_fence_is_signaled(f))
>   ts = dma_fence_timestamp(f)
> 
> could misorder the accesses to the signaled-bit.
> 
> 
> Do you have another example of what would break?

Please search the mailing list. We had the same patch like three or four times already.

It was even applied one IIRC, but yes it immediately breaks things.

Regards,
Christian.

> 
> 
> You see, the bit sometimes being locked and sometimes being lockless
> causes confusion.
> 
> 
> P.

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[kernel test robot]

Hi Philipp,

kernel test robot noticed the following build errors:

[auto build test ERROR on 1ecab8b0bad393750c60cc3819ce9c256595463c]

url:    https://github.com/intel-lab-lkp/linux/commits/Philipp-Stanner/asm-generic-bitops-Add-set_bit_release/20261021-115512
base:   1ecab8b0bad393750c60cc3819ce9c256595463c
patch link:    https://lore.kernel.org/r/20261021095512.1345191-4-phasta%40kernel.org
patch subject: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20261026/202610260958.IcV50YNq-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 16.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261026/202610260958.IcV50YNq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610260958.IcV50YNq-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/dma-buf/dma-fence.c: In function 'dma_fence_signal_timestamp_locked':
>> drivers/dma-buf/dma-fence.c:393:9: error: implicit declaration of function 'set_bit_release'; did you mean 'seq_release'? [-Wimplicit-function-declaration]
     393 |         set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
         |         ^~~~~~~~~~~~~~~
         |         seq_release


vim +393 drivers/dma-buf/dma-fence.c

   337	
   338	/**
   339	 * dma_fence_signal_timestamp_locked - signal completion of a fence
   340	 * @fence: the fence to signal
   341	 * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
   342	 *
   343	 * Signal completion for software callbacks on a fence, this will unblock
   344	 * dma_fence_wait() calls and run all the callbacks added with
   345	 * dma_fence_add_callback(). Can be called multiple times, but since a fence
   346	 * can only go from the unsignaled to the signaled state and not back, it will
   347	 * only be effective the first time. Set the timestamp provided as the fence
   348	 * signal timestamp.
   349	 *
   350	 * Unlike dma_fence_signal_timestamp(), this function must be called with
   351	 * &dma_fence.lock held.
   352	 */
   353	void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
   354					      ktime_t timestamp)
   355	{
   356		const struct dma_fence_ops *ops;
   357		struct dma_fence_cb *cur, *tmp;
   358		struct list_head cb_list;
   359	
   360		dma_fence_assert_held(fence);
   361	
   362		if (unlikely(dma_fence_test_signaled_flag(fence)))
   363			return;
   364	
   365		trace_dma_fence_signaled(fence);
   366	
   367		/*
   368		 * When neither a release nor a wait operation is specified set the ops
   369		 * pointer to NULL to allow the fence structure to become independent
   370		 * from who originally issued it.
   371		 */
   372		ops = rcu_dereference_protected(fence->ops, true);
   373		if (!ops->release && !ops->wait)
   374			RCU_INIT_POINTER(fence->ops, NULL);
   375	
   376		/* Stash the cb_list before replacing it with the timestamp */
   377		list_replace(&fence->cb_list, &cb_list);
   378	
   379		fence->timestamp = timestamp;
   380		set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
   381	
   382		list_for_each_entry_safe(cur, tmp, &cb_list, node) {
   383			INIT_LIST_HEAD(&cur->node);
   384			cur->func(fence, cur);
   385		}
   386	
   387		/*
   388		 * This orders the signaled-state with the test_bit_acquire() in
   389		 * dma_fence_test_signaled_flag(). This ensures that functions like
   390		 * dma_fence_is_signaled() can only ever return true once signaling is
   391		 * completed (i.e., after the callbacks above ran).
   392		 */
 > 393		set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
   394	}
   395	EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
   396	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly

[kernel test robot]

Hi Philipp,

kernel test robot noticed the following build errors:

[auto build test ERROR on 1ecab8b0bad393750c60cc3819ce9c256595463c]

url:    https://github.com/intel-lab-lkp/linux/commits/Philipp-Stanner/asm-generic-bitops-Add-set_bit_release/20261021-115512
base:   1ecab8b0bad393750c60cc3819ce9c256595463c
patch link:    https://lore.kernel.org/r/20261021095512.1345191-4-phasta%40kernel.org
patch subject: [PATCH 2/2] dma-buf/dma-fence: Order setting of the signaled-bit properly
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20261026/202610261518.t3m9Kwds-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261026/202610261518.t3m9Kwds-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610261518.t3m9Kwds-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-fence.c:393:2: error: call to undeclared function 'set_bit_release'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     393 |         set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
         |         ^
   1 error generated.


vim +/set_bit_release +393 drivers/dma-buf/dma-fence.c

   337	
   338	/**
   339	 * dma_fence_signal_timestamp_locked - signal completion of a fence
   340	 * @fence: the fence to signal
   341	 * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
   342	 *
   343	 * Signal completion for software callbacks on a fence, this will unblock
   344	 * dma_fence_wait() calls and run all the callbacks added with
   345	 * dma_fence_add_callback(). Can be called multiple times, but since a fence
   346	 * can only go from the unsignaled to the signaled state and not back, it will
   347	 * only be effective the first time. Set the timestamp provided as the fence
   348	 * signal timestamp.
   349	 *
   350	 * Unlike dma_fence_signal_timestamp(), this function must be called with
   351	 * &dma_fence.lock held.
   352	 */
   353	void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
   354					      ktime_t timestamp)
   355	{
   356		const struct dma_fence_ops *ops;
   357		struct dma_fence_cb *cur, *tmp;
   358		struct list_head cb_list;
   359	
   360		dma_fence_assert_held(fence);
   361	
   362		if (unlikely(dma_fence_test_signaled_flag(fence)))
   363			return;
   364	
   365		trace_dma_fence_signaled(fence);
   366	
   367		/*
   368		 * When neither a release nor a wait operation is specified set the ops
   369		 * pointer to NULL to allow the fence structure to become independent
   370		 * from who originally issued it.
   371		 */
   372		ops = rcu_dereference_protected(fence->ops, true);
   373		if (!ops->release && !ops->wait)
   374			RCU_INIT_POINTER(fence->ops, NULL);
   375	
   376		/* Stash the cb_list before replacing it with the timestamp */
   377		list_replace(&fence->cb_list, &cb_list);
   378	
   379		fence->timestamp = timestamp;
   380		set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
   381	
   382		list_for_each_entry_safe(cur, tmp, &cb_list, node) {
   383			INIT_LIST_HEAD(&cur->node);
   384			cur->func(fence, cur);
   385		}
   386	
   387		/*
   388		 * This orders the signaled-state with the test_bit_acquire() in
   389		 * dma_fence_test_signaled_flag(). This ensures that functions like
   390		 * dma_fence_is_signaled() can only ever return true once signaling is
   391		 * completed (i.e., after the callbacks above ran).
   392		 */
 > 393		set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
   394	}
   395	EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
   396	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki