[PATCH 09/12] unwind: Make unwind_task_info::unwind_mask consistent

Peter Zijlstra posted 12 patches 1 week ago
[PATCH 09/12] unwind: Make unwind_task_info::unwind_mask consistent
Posted by Peter Zijlstra 1 week ago
The unwind_task_info::unwind_mask was manipulated using a mixture of:

  regular store
  WRITE_ONCE()
  try_cmpxchg()
  set_bit()
  atomic_long_*()

Clean up and make it consistently atomic_long_t.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/unwind_deferred.h       |    4 ++--
 include/linux/unwind_deferred_types.h |    3 ++-
 kernel/unwind/deferred.c              |   17 +++++++++--------
 3 files changed, 13 insertions(+), 11 deletions(-)

--- a/include/linux/unwind_deferred.h
+++ b/include/linux/unwind_deferred.h
@@ -46,7 +46,7 @@ void unwind_deferred_task_exit(struct ta
 static __always_inline void unwind_reset_info(void)
 {
 	struct unwind_task_info *info = &current->unwind_info;
-	unsigned long bits = info->unwind_mask;
+	unsigned long bits = atomic_long_read(&info->unwind_mask);
 
 	/* Was there any unwinding? */
 	if (likely(!bits))
@@ -56,7 +56,7 @@ static __always_inline void unwind_reset
 		/* Is a task_work going to run again before going back */
 		if (bits & UNWIND_PENDING)
 			return;
-	} while (!try_cmpxchg(&info->unwind_mask, &bits, 0UL));
+	} while (!atomic_long_try_cmpxchg(&info->unwind_mask, &bits, 0UL));
 	current->unwind_info.id.id = 0;
 
 	if (unlikely(info->cache)) {
--- a/include/linux/unwind_deferred_types.h
+++ b/include/linux/unwind_deferred_types.h
@@ -3,6 +3,7 @@
 #define _LINUX_UNWIND_USER_DEFERRED_TYPES_H
 
 #include <linux/types.h>
+#include <linux/atomic.h>
 
 struct unwind_cache {
 	unsigned long		unwind_completed;
@@ -32,7 +33,7 @@ union unwind_task_id {
 };
 
 struct unwind_task_info {
-	unsigned long		unwind_mask;
+	atomic_long_t		unwind_mask;
 	struct unwind_cache	*cache;
 	struct callback_head	work;
 	union unwind_task_id	id;
--- a/kernel/unwind/deferred.c
+++ b/kernel/unwind/deferred.c
@@ -53,7 +53,7 @@ DEFINE_STATIC_SRCU(unwind_srcu);
 
 static inline bool unwind_pending(struct unwind_task_info *info)
 {
-	return test_bit(UNWIND_PENDING_BIT, &info->unwind_mask);
+	return atomic_long_read(&info->unwind_mask) & UNWIND_USED;
 }
 
 /*
@@ -142,7 +142,7 @@ int unwind_user_faultable(struct unwind_
 	cache->nr_entries = trace->nr;
 
 	/* Clear nr_entries on way back to user space */
-	set_bit(UNWIND_USED_BIT, &info->unwind_mask);
+	atomic_long_or(UNWIND_USED, &info->unwind_mask);
 
 	return 0;
 }
@@ -160,7 +160,7 @@ static void process_unwind_deferred(stru
 
 	/* Clear pending bit but make sure to have the current bits */
 	bits = atomic_long_fetch_andnot(UNWIND_PENDING,
-				  (atomic_long_t *)&info->unwind_mask);
+					&info->unwind_mask);
 	/*
 	 * From here on out, the callback must always be called, even if it's
 	 * just an empty trace.
@@ -265,7 +265,7 @@ int unwind_deferred_request(struct unwin
 
 	*cookie = get_cookie(info);
 
-	old = READ_ONCE(info->unwind_mask);
+	old = atomic_long_read(&info->unwind_mask);
 
 	/* Is this already queued or executed */
 	if (old & bit)
@@ -278,7 +278,7 @@ int unwind_deferred_request(struct unwin
 	 * to have a callback.
 	 */
 	bits = UNWIND_PENDING | bit;
-	old = atomic_long_fetch_or(bits, (atomic_long_t *)&info->unwind_mask);
+	old = atomic_long_fetch_or(bits, &info->unwind_mask);
 	if (old & bits) {
 		/*
 		 * If the work's bit was set, whatever set it had better
@@ -292,7 +292,7 @@ int unwind_deferred_request(struct unwin
 	ret = task_work_add(current, &info->work, twa_mode);
 
 	if (WARN_ON_ONCE(ret))
-		WRITE_ONCE(info->unwind_mask, 0);
+		atomic_long_set(&info->unwind_mask, 0);
 
 	return ret;
 }
@@ -324,7 +324,8 @@ void unwind_deferred_cancel(struct unwin
 	guard(rcu)();
 	/* Clear this bit from all threads */
 	for_each_process_thread(g, t) {
-		clear_bit(bit, &t->unwind_info.unwind_mask);
+		atomic_long_andnot(UNWIND_USED,
+				   &t->unwind_info.unwind_mask);
 		if (t->unwind_info.cache)
 			clear_bit(bit, &t->unwind_info.cache->unwind_completed);
 	}
@@ -354,7 +355,7 @@ void unwind_task_init(struct task_struct
 
 	memset(info, 0, sizeof(*info));
 	init_task_work(&info->work, unwind_deferred_task_work);
-	info->unwind_mask = 0;
+	atomic_long_set(&info->unwind_mask, 0);
 }
 
 void unwind_task_free(struct task_struct *task)
Re: [PATCH 09/12] unwind: Make unwind_task_info::unwind_mask consistent
Posted by Steven Rostedt 10 hours ago
On Wed, 24 Sep 2025 09:59:57 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> @@ -324,7 +324,8 @@ void unwind_deferred_cancel(struct unwin
>  	guard(rcu)();
>  	/* Clear this bit from all threads */
>  	for_each_process_thread(g, t) {
> -		clear_bit(bit, &t->unwind_info.unwind_mask);
> +		atomic_long_andnot(UNWIND_USED,
> +				   &t->unwind_info.unwind_mask);

Shouldn't this be:

		atomic_long_andnot(BIT(bit), &t->unwind_info.unwind_mask);

 ?

As BIT(bit) != UNWIND_USED.

-- Steve

>  		if (t->unwind_info.cache)
>  			clear_bit(bit, &t->unwind_info.cache->unwind_completed);
>  	}