Re: [PATCH] tracing/user_events: Run BPF program if attached
From: Beau Belgrave <hidden>
Date: 2023-05-17 23:01:10
Also in:
bpf, lkml
Subsystem:
the rest, tracing · Maintainers:
Linus Torvalds, Steven Rostedt, Masami Hiramatsu
On Wed, May 17, 2023 at 12:37:11PM -0700, Linus Torvalds wrote:
On Wed, May 17, 2023 at 12:36 PM Linus Torvalds [off-list ref] wrote:quoted
.. this is the patch that I think should go on top of it to fix the misleading "safe" and the incorrect RCU walk.Let's actually attach the patch too. Duh. Linus
quoted hunk ↗ jump to hunk
kernel/trace/trace_events_user.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c index b2aecbfbbd24..054e28cc5ad4 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c@@ -439,7 +439,7 @@ static bool user_event_enabler_exists(struct user_event_mm *mm, struct user_event_enabler *enabler; struct user_event_enabler *next; - list_for_each_entry_safe(enabler, next, &mm->enablers, link) { + list_for_each_entry(enabler, next, &mm->enablers, link) { if (enabler->addr == uaddr && (enabler->values & ENABLE_VAL_BIT_MASK) == bit) return true;@@ -455,19 +455,19 @@ static void user_event_enabler_update(struct user_event *user) struct user_event_mm *next; int attempt; + lockdep_assert_held(&event_mutex); + while (mm) { next = mm->next; mmap_read_lock(mm->mm); - rcu_read_lock(); - list_for_each_entry_rcu(enabler, &mm->enablers, link) { + list_for_each_entry(enabler, &mm->enablers, link) { if (enabler->event == user) { attempt = 0; user_event_enabler_write(mm, enabler, true, &attempt); } } - rcu_read_unlock(); mmap_read_unlock(mm->mm); user_event_mm_put(mm); mm = next;
Do you mind giving me your Signed-off-by for these? I plan to do a series where I take these patches and then also fix up a few comments and the link namings as you suggested. First patch is clean, second patch I made the following changes and after that passed all the self-tests without bug splats with CONFIG_PROVE_LOCKING/RCU and ATOMIC_SLEEP:
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index b2aecbfbbd24..2f70dabb0f71 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c@@ -437,9 +437,8 @@ static bool user_event_enabler_exists(struct user_event_mm *mm, unsigned long uaddr, unsigned char bit) { struct user_event_enabler *enabler; - struct user_event_enabler *next; - list_for_each_entry_safe(enabler, next, &mm->enablers, link) { + list_for_each_entry(enabler, &mm->enablers, link) { if (enabler->addr == uaddr && (enabler->values & ENABLE_VAL_BIT_MASK) == bit) return true;
@@ -495,7 +494,9 @@ static bool user_event_enabler_dup(struct user_event_enabler *orig, enabler->values = orig->values & ENABLE_VAL_DUP_MASK; refcount_inc(&enabler->event->refcnt); - list_add_rcu(&enabler->link, &mm->enablers); + + /* Enablers not exposed yet, RCU not required */ + list_add(&enabler->link, &mm->enablers);