Skip to content

Commit

Permalink
tracing/user_events: Rename link fields for clarity
Browse files Browse the repository at this point in the history 
Currently most list_head fields of various structs within user_events
are simply named link. This causes folks to keep additional context in
their head when working with the code, which can be confusing.

Instead of using link, describe what the actual link is, for example:
list_del_rcu(&mm->link);

Changes into:
list_del_rcu(&mm->mms_link);

The reader now is given a hint the link is to the mms global list
instead of having to remember or spot check within the code.

Link: https://lkml.kernel.org/r/[email protected]
Link: https://lore.kernel.org/linux-trace-kernel/CAHk-=wicngggxVpbnrYHjRTwGE0WYscPRM+L2HO2BF8ia1EXgQ@mail.gmail.com/

Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Beau Belgrave <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
  • Loading branch information
@beaubelgrave @rostedt
beaubelgrave authored and rostedt committed May 24, 2023
1 parent aaecdaf commit dcbd1ac
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 19 deletions.
2 changes: 1 addition & 1 deletion include/linux/user_events.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#ifdef CONFIG_USER_EVENTS
struct user_event_mm {
struct list_head link;
struct list_head mms_link;
struct list_head enablers;
struct mm_struct *mm;
struct user_event_mm *next;
Expand Down
40 changes: 22 additions & 18 deletions kernel/trace/trace_events_user.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ struct user_event {
* these to track enablement sites that are tied to an event.
*/
struct user_event_enabler {
struct list_head link;
struct list_head mm_enablers_link;
struct user_event *event;
unsigned long addr;

Expand Down Expand Up @@ -155,7 +155,7 @@ struct user_event_file_info {
#define VALIDATOR_REL (1 << 1)

struct user_event_validator {
struct list_head link;
struct list_head user_event_link;
int offset;
int flags;
};
Expand Down Expand Up @@ -261,7 +261,7 @@ static struct user_event_group

static void user_event_enabler_destroy(struct user_event_enabler *enabler)
{
list_del_rcu(&enabler->link);
list_del_rcu(&enabler->mm_enablers_link);

/* No longer tracking the event via the enabler */
refcount_dec(&enabler->event->refcnt);
Expand Down Expand Up @@ -440,7 +440,7 @@ static bool user_event_enabler_exists(struct user_event_mm *mm,
{
struct user_event_enabler *enabler;

list_for_each_entry(enabler, &mm->enablers, link) {
list_for_each_entry(enabler, &mm->enablers, mm_enablers_link) {
if (enabler->addr == uaddr && ENABLE_BIT(enabler) == bit)
return true;
}
Expand All @@ -461,7 +461,7 @@ static void user_event_enabler_update(struct user_event *user)
next = mm->next;
mmap_read_lock(mm->mm);

list_for_each_entry(enabler, &mm->enablers, link) {
list_for_each_entry(enabler, &mm->enablers, mm_enablers_link) {
if (enabler->event == user) {
attempt = 0;
user_event_enabler_write(mm, enabler, true, &attempt);
Expand Down Expand Up @@ -497,7 +497,7 @@ static bool user_event_enabler_dup(struct user_event_enabler *orig,
refcount_inc(&enabler->event->refcnt);

/* Enablers not exposed yet, RCU not required */
list_add(&enabler->link, &mm->enablers);
list_add(&enabler->mm_enablers_link, &mm->enablers);

return true;
}
Expand Down Expand Up @@ -527,13 +527,15 @@ static struct user_event_mm *user_event_mm_get_all(struct user_event *user)
*/
rcu_read_lock();

list_for_each_entry_rcu(mm, &user_event_mms, link)
list_for_each_entry_rcu(enabler, &mm->enablers, link)
list_for_each_entry_rcu(mm, &user_event_mms, mms_link) {
list_for_each_entry_rcu(enabler, &mm->enablers, mm_enablers_link) {
if (enabler->event == user) {
mm->next = found;
found = user_event_mm_get(mm);
break;
}
}
}

rcu_read_unlock();

Expand Down Expand Up @@ -572,7 +574,7 @@ static void user_event_mm_attach(struct user_event_mm *user_mm, struct task_stru
unsigned long flags;

spin_lock_irqsave(&user_event_mms_lock, flags);
list_add_rcu(&user_mm->link, &user_event_mms);
list_add_rcu(&user_mm->mms_link, &user_event_mms);
spin_unlock_irqrestore(&user_event_mms_lock, flags);

t->user_event_mm = user_mm;
Expand Down Expand Up @@ -601,7 +603,7 @@ static void user_event_mm_destroy(struct user_event_mm *mm)
{
struct user_event_enabler *enabler, *next;

list_for_each_entry_safe(enabler, next, &mm->enablers, link)
list_for_each_entry_safe(enabler, next, &mm->enablers, mm_enablers_link)
user_event_enabler_destroy(enabler);

mmdrop(mm->mm);
Expand Down Expand Up @@ -638,7 +640,7 @@ void user_event_mm_remove(struct task_struct *t)

/* Remove the mm from the list, so it can no longer be enabled */
spin_lock_irqsave(&user_event_mms_lock, flags);
list_del_rcu(&mm->link);
list_del_rcu(&mm->mms_link);
spin_unlock_irqrestore(&user_event_mms_lock, flags);

/*
Expand Down Expand Up @@ -686,9 +688,10 @@ void user_event_mm_dup(struct task_struct *t, struct user_event_mm *old_mm)

rcu_read_lock();

list_for_each_entry_rcu(enabler, &old_mm->enablers, link)
list_for_each_entry_rcu(enabler, &old_mm->enablers, mm_enablers_link) {
if (!user_event_enabler_dup(enabler, mm))
goto error;
}

rcu_read_unlock();

Expand Down Expand Up @@ -757,7 +760,7 @@ static struct user_event_enabler
*/
if (!*write_result) {
refcount_inc(&enabler->event->refcnt);
list_add_rcu(&enabler->link, &user_mm->enablers);
list_add_rcu(&enabler->mm_enablers_link, &user_mm->enablers);
}

mutex_unlock(&event_mutex);
Expand Down Expand Up @@ -913,8 +916,8 @@ static void user_event_destroy_validators(struct user_event *user)
struct user_event_validator *validator, *next;
struct list_head *head = &user->validators;

list_for_each_entry_safe(validator, next, head, link) {
list_del(&validator->link);
list_for_each_entry_safe(validator, next, head, user_event_link) {
list_del(&validator->user_event_link);
kfree(validator);
}
}
Expand Down Expand Up @@ -968,7 +971,7 @@ static int user_event_add_field(struct user_event *user, const char *type,
validator->offset = offset;

/* Want sequential access when validating */
list_add_tail(&validator->link, &user->validators);
list_add_tail(&validator->user_event_link, &user->validators);

add_field:
field->type = type;
Expand Down Expand Up @@ -1358,7 +1361,7 @@ static int user_event_validate(struct user_event *user, void *data, int len)
void *pos, *end = data + len;
u32 loc, offset, size;

list_for_each_entry(validator, head, link) {
list_for_each_entry(validator, head, user_event_link) {
pos = data + validator->offset;

/* Already done min_size check, no bounds check here */
Expand Down Expand Up @@ -2279,7 +2282,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
*/
mutex_lock(&event_mutex);

list_for_each_entry_safe(enabler, next, &mm->enablers, link)
list_for_each_entry_safe(enabler, next, &mm->enablers, mm_enablers_link) {
if (enabler->addr == reg.disable_addr &&
ENABLE_BIT(enabler) == reg.disable_bit) {
set_bit(ENABLE_VAL_FREEING_BIT, ENABLE_BITOPS(enabler));
Expand All @@ -2290,6 +2293,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
/* Removed at least one */
ret = 0;
}
}

mutex_unlock(&event_mutex);

Expand Down

0 comments on commit dcbd1ac

Please sign in to comment.