torvalds/linux
torvalds/linux
2
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-09-04 20:19:47 +08:00
Code

rv: Merge struct rv_reactor_def into struct rv_reactor

Browse Source 
Each struct rv_reactor has a unique struct rv_reactor_def associated with
it. struct rv_reactor is statically allocated, while struct rv_reactor_def
is dynamically allocated.

This makes the code more complicated than it should be:

  - Lookup is required to get the associated rv_reactor_def from rv_reactor

  - Dynamic memory allocation is required for rv_reactor_def. This is
    harder to get right compared to static memory. For instance, there is
    an existing mistake: rv_unregister_reactor() does not free the memory
    allocated by rv_register_reactor(). This is fortunately not a real
    memory leak problem as rv_unregister_reactor() is never called.

Simplify and merge rv_reactor_def into rv_reactor.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/71cb91c86cd40df5b8c492b788787f2a73c3eaa3.1753378331.git.namcao@linutronix.de
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This commit is contained in:
Nam Cao 2025-07-24 19:33:28 +02:00 committed by Steven Rostedt (Google)
parent 24cbfe18d5
commit 3d3c376118
3 changed files with 43 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/linux/rv.h
View File

@ -90,6 +90,9 @@ struct rv_reactor {
const char *name; const char *name;
const char *description; const char *description;
__printf(1, 2) void (*react)(const char *msg, ...); __printf(1, 2) void (*react)(const char *msg, ...);
struct list_head list;
/* protected by the monitor interface lock */
int counter;
}; };
#endif #endif
@ -101,7 +104,7 @@ struct rv_monitor {
void (*disable)(void); void (*disable)(void);
void (*reset)(void); void (*reset)(void);
#ifdef CONFIG_RV_REACTORS #ifdef CONFIG_RV_REACTORS
struct rv_reactor_def *rdef; struct rv_reactor *reactor;
__printf(1, 2) void (*react)(const char *msg, ...); __printf(1, 2) void (*react)(const char *msg, ...);
bool reacting; bool reacting;
#endif #endif

9
kernel/trace/rv/rv.h
View File

@ -23,15 +23,6 @@ struct rv_interface {
extern struct mutex rv_interface_lock; extern struct mutex rv_interface_lock;
extern struct list_head rv_monitors_list; extern struct list_head rv_monitors_list;
#ifdef CONFIG_RV_REACTORS
struct rv_reactor_def {
struct list_head list;
struct rv_reactor *reactor;
/* protected by the monitor interface lock */
int counter;
};
#endif
struct dentry *get_monitors_root(void); struct dentry *get_monitors_root(void);
int rv_disable_monitor(struct rv_monitor *mon); int rv_disable_monitor(struct rv_monitor *mon);
int rv_enable_monitor(struct rv_monitor *mon); int rv_enable_monitor(struct rv_monitor *mon);

92
kernel/trace/rv/rv_reactors.c
View File

@ -70,12 +70,12 @@
*/ */
static LIST_HEAD(rv_reactors_list); static LIST_HEAD(rv_reactors_list);
static struct rv_reactor_def *get_reactor_rdef_by_name(char *name) static struct rv_reactor *get_reactor_rdef_by_name(char *name)
{ {
struct rv_reactor_def *r; struct rv_reactor *r;
list_for_each_entry(r, &rv_reactors_list, list) { list_for_each_entry(r, &rv_reactors_list, list) {
if (strcmp(name, r->reactor->name) == 0) if (strcmp(name, r->name) == 0)
return r; return r;
} }
return NULL; return NULL;
@ -86,9 +86,9 @@ static struct rv_reactor_def *get_reactor_rdef_by_name(char *name)
*/ */
static int reactors_show(struct seq_file *m, void *p) static int reactors_show(struct seq_file *m, void *p)
{ {
struct rv_reactor_def *rea_def = p; struct rv_reactor *reactor = p;
seq_printf(m, "%s\n", rea_def->reactor->name); seq_printf(m, "%s\n", reactor->name);
return 0; return 0;
} }
@ -139,12 +139,12 @@ static const struct file_operations available_reactors_ops = {
static int monitor_reactor_show(struct seq_file *m, void *p) static int monitor_reactor_show(struct seq_file *m, void *p)
{ {
struct rv_monitor *mon = m->private; struct rv_monitor *mon = m->private;
struct rv_reactor_def *rdef = p; struct rv_reactor *reactor = p;
if (mon->rdef == rdef) if (mon->reactor == reactor)
seq_printf(m, "[%s]\n", rdef->reactor->name); seq_printf(m, "[%s]\n", reactor->name);
else else
seq_printf(m, "%s\n", rdef->reactor->name); seq_printf(m, "%s\n", reactor->name);
return 0; return 0;
} }
@ -159,13 +159,13 @@ static const struct seq_operations monitor_reactors_seq_ops = {
}; };
static void monitor_swap_reactors_single(struct rv_monitor *mon, static void monitor_swap_reactors_single(struct rv_monitor *mon,
struct rv_reactor_def *rdef, struct rv_reactor *reactor,
bool reacting, bool nested) bool reacting, bool nested)
{ {
bool monitor_enabled; bool monitor_enabled;
/* nothing to do */ /* nothing to do */
if (mon->rdef == rdef) if (mon->reactor == reactor)
return; return;
monitor_enabled = mon->enabled; monitor_enabled = mon->enabled;
@ -173,12 +173,12 @@ static void monitor_swap_reactors_single(struct rv_monitor *mon,
rv_disable_monitor(mon); rv_disable_monitor(mon);
/* swap reactor's usage */ /* swap reactor's usage */
mon->rdef->counter--; mon->reactor->counter--;
rdef->counter++; reactor->counter++;
mon->rdef = rdef; mon->reactor = reactor;
mon->reacting = reacting; mon->reacting = reacting;
mon->react = rdef->reactor->react; mon->react = reactor->react;
/* enable only once if iterating through a container */ /* enable only once if iterating through a container */
if (monitor_enabled && !nested) if (monitor_enabled && !nested)
@ -186,7 +186,7 @@ static void monitor_swap_reactors_single(struct rv_monitor *mon,
} }
static void monitor_swap_reactors(struct rv_monitor *mon, static void monitor_swap_reactors(struct rv_monitor *mon,
struct rv_reactor_def *rdef, bool reacting) struct rv_reactor *reactor, bool reacting)
{ {
struct rv_monitor *p = mon; struct rv_monitor *p = mon;
@ -194,7 +194,7 @@ static void monitor_swap_reactors(struct rv_monitor *mon,
list_for_each_entry_continue(p, &rv_monitors_list, list) { list_for_each_entry_continue(p, &rv_monitors_list, list) {
if (p->parent != mon) if (p->parent != mon)
break; break;
monitor_swap_reactors_single(p, rdef, reacting, true); monitor_swap_reactors_single(p, reactor, reacting, true);
} }
/* /*
* This call enables and disables the monitor if they were active. * This call enables and disables the monitor if they were active.
@ -202,7 +202,7 @@ static void monitor_swap_reactors(struct rv_monitor *mon,
* All nested monitors are enabled also if they were off, we may refine * All nested monitors are enabled also if they were off, we may refine
* this logic in the future. * this logic in the future.
*/ */
monitor_swap_reactors_single(mon, rdef, reacting, false); monitor_swap_reactors_single(mon, reactor, reacting, false);
} }
static ssize_t static ssize_t
@ -211,7 +211,7 @@ monitor_reactors_write(struct file *file, const char __user *user_buf,
{ {
char buff[MAX_RV_REACTOR_NAME_SIZE + 2]; char buff[MAX_RV_REACTOR_NAME_SIZE + 2];
struct rv_monitor *mon; struct rv_monitor *mon;
struct rv_reactor_def *rdef; struct rv_reactor *reactor;
struct seq_file *seq_f; struct seq_file *seq_f;
int retval = -EINVAL; int retval = -EINVAL;
bool enable; bool enable;
@ -243,16 +243,16 @@ monitor_reactors_write(struct file *file, const char __user *user_buf,
retval = -EINVAL; retval = -EINVAL;
list_for_each_entry(rdef, &rv_reactors_list, list) { list_for_each_entry(reactor, &rv_reactors_list, list) {
if (strcmp(ptr, rdef->reactor->name) != 0) if (strcmp(ptr, reactor->name) != 0)
continue; continue;
if (rdef == get_reactor_rdef_by_name("nop")) if (strcmp(reactor->name, "nop"))
enable = false; enable = false;
else else
enable = true; enable = true;
monitor_swap_reactors(mon, rdef, enable); monitor_swap_reactors(mon, reactor, enable);
retval = count; retval = count;
break; break;
@ -299,23 +299,16 @@ static const struct file_operations monitor_reactors_ops = {
static int __rv_register_reactor(struct rv_reactor *reactor) static int __rv_register_reactor(struct rv_reactor *reactor)
{ {
struct rv_reactor_def *r; struct rv_reactor *r;
list_for_each_entry(r, &rv_reactors_list, list) { list_for_each_entry(r, &rv_reactors_list, list) {
if (strcmp(reactor->name, r->reactor->name) == 0) { if (strcmp(reactor->name, r->name) == 0) {
pr_info("Reactor %s is already registered\n", reactor->name); pr_info("Reactor %s is already registered\n", reactor->name);
return -EINVAL; return -EINVAL;
} }
} }
r = kzalloc(sizeof(struct rv_reactor_def), GFP_KERNEL); list_add_tail(&reactor->list, &rv_reactors_list);
if (!r)
return -ENOMEM;
r->reactor = reactor;
r->counter = 0;
list_add_tail(&r->list, &rv_reactors_list);
return 0; return 0;
} }
@ -350,26 +343,19 @@ int rv_register_reactor(struct rv_reactor *reactor)
*/ */
int rv_unregister_reactor(struct rv_reactor *reactor) int rv_unregister_reactor(struct rv_reactor *reactor)
{ {
struct rv_reactor_def *ptr, *next;
int ret = 0; int ret = 0;
mutex_lock(&rv_interface_lock); mutex_lock(&rv_interface_lock);
list_for_each_entry_safe(ptr, next, &rv_reactors_list, list) { if (!reactor->counter) {
if (strcmp(reactor->name, ptr->reactor->name) == 0) { list_del(&reactor->list);
} else {
if (!ptr->counter) { printk(KERN_WARNING
list_del(&ptr->list); "rv: the rv_reactor %s is in use by %d monitor(s)\n",
} else { reactor->name, reactor->counter);
printk(KERN_WARNING printk(KERN_WARNING "rv: the rv_reactor %s cannot be removed\n",
"rv: the rv_reactor %s is in use by %d monitor(s)\n", reactor->name);
ptr->reactor->name, ptr->counter); ret = -EBUSY;
printk(KERN_WARNING "rv: the rv_reactor %s cannot be removed\n",
ptr->reactor->name);
ret = -EBUSY;
break;
}
}
} }
mutex_unlock(&rv_interface_lock); mutex_unlock(&rv_interface_lock);
@ -469,8 +455,8 @@ int reactor_populate_monitor(struct rv_monitor *mon)
/* /*
* Configure as the rv_nop reactor. * Configure as the rv_nop reactor.
*/ */
mon->rdef = get_reactor_rdef_by_name("nop"); mon->reactor = get_reactor_rdef_by_name("nop");
mon->rdef->counter++; mon->reactor->counter++;
mon->reacting = false; mon->reacting = false;
return 0; return 0;
@ -483,8 +469,8 @@ int reactor_populate_monitor(struct rv_monitor *mon)
void reactor_cleanup_monitor(struct rv_monitor *mon) void reactor_cleanup_monitor(struct rv_monitor *mon)
{ {
lockdep_assert_held(&rv_interface_lock); lockdep_assert_held(&rv_interface_lock);
mon->rdef->counter--; mon->reactor->counter--;
WARN_ON_ONCE(mon->rdef->counter < 0); WARN_ON_ONCE(mon->reactor->counter < 0);
} }
/* /*