Skip to content

Commit ff9e163

Browse files
beaubelgraverostedt
authored andcommitted
tracing/user_events: Document user_event_mm one-shot list usage
During 6.4 development it became clear that the one-shot list used by the user_event_mm's next field was confusing to others. It is not clear how this list is protected or what the next field usage is for unless you are familiar with the code. Add comments into the user_event_mm struct indicating lock requirement and usage. Also document how and why this approach was used via comments in both user_event_enabler_update() and user_event_mm_get_all() and the rules to properly use it. Link: https://lkml.kernel.org/r/20230519230741.669-5-beaub@linux.microsoft.com Link: https://lore.kernel.org/linux-trace-kernel/CAHk-=wicngggxVpbnrYHjRTwGE0WYscPRM+L2HO2BF8ia1EXgQ@mail.gmail.com/ Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent dcbd1ac commit ff9e163

2 files changed

Lines changed: 23 additions & 1 deletion

File tree

include/linux/user_events.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ struct user_event_mm {
2020
struct list_head mms_link;
2121
struct list_head enablers;
2222
struct mm_struct *mm;
23+
/* Used for one-shot lists, protected by event_mutex */
2324
struct user_event_mm *next;
2425
refcount_t refcnt;
2526
refcount_t tasks;

kernel/trace/trace_events_user.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,12 +451,25 @@ static bool user_event_enabler_exists(struct user_event_mm *mm,
451451
static void user_event_enabler_update(struct user_event *user)
452452
{
453453
struct user_event_enabler *enabler;
454-
struct user_event_mm *mm = user_event_mm_get_all(user);
455454
struct user_event_mm *next;
455+
struct user_event_mm *mm;
456456
int attempt;
457457

458458
lockdep_assert_held(&event_mutex);
459459

460+
/*
461+
* We need to build a one-shot list of all the mms that have an
462+
* enabler for the user_event passed in. This list is only valid
463+
* while holding the event_mutex. The only reason for this is due
464+
* to the global mm list being RCU protected and we use methods
465+
* which can wait (mmap_read_lock and pin_user_pages_remote).
466+
*
467+
* NOTE: user_event_mm_get_all() increments the ref count of each
468+
* mm that is added to the list to prevent removal timing windows.
469+
* We must always put each mm after they are used, which may wait.
470+
*/
471+
mm = user_event_mm_get_all(user);
472+
460473
while (mm) {
461474
next = mm->next;
462475
mmap_read_lock(mm->mm);
@@ -515,6 +528,14 @@ static struct user_event_mm *user_event_mm_get_all(struct user_event *user)
515528
struct user_event_enabler *enabler;
516529
struct user_event_mm *mm;
517530

531+
/*
532+
* We use the mm->next field to build a one-shot list from the global
533+
* RCU protected list. To build this list the event_mutex must be held.
534+
* This lets us build a list without requiring allocs that could fail
535+
* when user based events are most wanted for diagnostics.
536+
*/
537+
lockdep_assert_held(&event_mutex);
538+
518539
/*
519540
* We do not want to block fork/exec while enablements are being
520541
* updated, so we use RCU to walk the current tasks that have used

0 commit comments

Comments
 (0)