Re: [PATCH v8 12/18] unwind deferred: Use SRCU unwind_deferred_task_work()

On Sat, May 10, 2025 at 6:41 AM Steven Rostedt [off-list ref] wrote:

On Fri, 9 May 2025 14:49:37 -0700
Andrii Nakryiko [off-list ref] wrote:

quoted

quoted

@@ -133,13 +135,15 @@ static void unwind_deferred_task_work(struct callback_head *head)

        timestamp = info->timestamp;

-       guard(mutex)(&callback_mutex);
-       list_for_each_entry(work, &callbacks, list) {
+       idx = srcu_read_lock(&unwind_srcu);

nit: you could have used guard(srcu)(&unwind_srcu) ?

Then it would be a scope_guard() as it is only needed for the list. I
prefer using guard() when it is most of the function that is being
protected. Here it's just the list and nothing else.

One issue I have with guard() is that it tends to "leak". That is, if you
use it to protect only one thing and then add more after what you are
protecting, then the guard ends up protecting more than it needs to.

Yep, makes sense. I just noticed the use of guard() for mutex before,
so assumes the same could be done for SRCU, but what you are saying
makes sense, no problem.

If anything, I would make the loop into its own function with the guard()
then it is more obvious. But for now, I think it's fine as is, unless
others prefer the switch.

-- Steve

quoted

quoted

+       list_for_each_entry_srcu(work, &callbacks, list,
+                                srcu_read_lock_held(&unwind_srcu)) {
                if (task->unwind_mask & (1UL << work->bit)) {
                        work->func(work, &trace, timestamp);
                        clear_bit(work->bit, &current->unwind_mask);
                }
        }
+       srcu_read_unlock(&unwind_srcu, idx);
 }