Commit 3d3c3761 authored by Nam Cao's avatar Nam Cao Committed by Steven Rostedt (Google)
Browse files

rv: Merge struct rv_reactor_def into struct rv_reactor 

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: default avatarGabriele Monaco <gmonaco@redhat.com>
Signed-off-by: default avatarNam Cao <namcao@linutronix.de>
Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
parent 24cbfe18

include/linux/rv.h

+4 −1
Original line number Diff line number Diff line
@@ -90,6 +90,9 @@ struct rv_reactor { 
	const char		*name;

	const char		*description;

	__printf(1, 2) void	(*react)(const char *msg, ...);

	struct list_head	list;

	/* protected by the monitor interface lock */

	int			counter;

};

#endif
@@ -101,7 +104,7 @@ struct rv_monitor { 
	void			(*disable)(void);

	void			(*reset)(void);

#ifdef CONFIG_RV_REACTORS

	struct rv_reactor_def	*rdef;

	struct rv_reactor	*reactor;

	__printf(1, 2) void	(*react)(const char *msg, ...);

	bool			reacting;

#endif

kernel/trace/rv/rv.h

+0 −9
Original line number Diff line number Diff line
@@ -23,15 +23,6 @@ struct rv_interface { 
extern struct mutex rv_interface_lock;

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);

int rv_disable_monitor(struct rv_monitor *mon);

int rv_enable_monitor(struct rv_monitor *mon);

kernel/trace/rv/rv_reactors.c

+39 −53
Original line number Diff line number Diff line
@@ -70,12 +70,12 @@ 
 */

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) {

		if (strcmp(name, r->reactor->name) == 0)

		if (strcmp(name, r->name) == 0)

			return r;

	}

	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)

{

	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;

}
@@ -139,12 +139,12 @@ static const struct file_operations available_reactors_ops = { 
static int monitor_reactor_show(struct seq_file *m, void *p)

{

	struct rv_monitor *mon = m->private;

	struct rv_reactor_def *rdef = p;

	struct rv_reactor *reactor = p;



	if (mon->rdef == rdef)

		seq_printf(m, "[%s]\n", rdef->reactor->name);

	if (mon->reactor == reactor)

		seq_printf(m, "[%s]\n", reactor->name);

	else

		seq_printf(m, "%s\n", rdef->reactor->name);

		seq_printf(m, "%s\n", reactor->name);

	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,

					 struct rv_reactor_def *rdef,

					 struct rv_reactor *reactor,

					 bool reacting, bool nested)

{

	bool monitor_enabled;



	/* nothing to do */

	if (mon->rdef == rdef)

	if (mon->reactor == reactor)

		return;



	monitor_enabled = mon->enabled;
@@ -173,12 +173,12 @@ static void monitor_swap_reactors_single(struct rv_monitor *mon, 
		rv_disable_monitor(mon);



	/* swap reactor's usage */

	mon->rdef->counter--;

	rdef->counter++;

	mon->reactor->counter--;

	reactor->counter++;



	mon->rdef = rdef;

	mon->reactor = reactor;

	mon->reacting = reacting;

	mon->react = rdef->reactor->react;

	mon->react = reactor->react;



	/* enable only once if iterating through a container */

	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,

				  struct rv_reactor_def *rdef, bool reacting)

				  struct rv_reactor *reactor, bool reacting)

{

	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) {

			if (p->parent != mon)

				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.
@@ -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

	 * this logic in the future.

	 */

	monitor_swap_reactors_single(mon, rdef, reacting, false);

	monitor_swap_reactors_single(mon, reactor, reacting, false);

}



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];

	struct rv_monitor *mon;

	struct rv_reactor_def *rdef;

	struct rv_reactor *reactor;

	struct seq_file *seq_f;

	int retval = -EINVAL;

	bool enable;
@@ -243,16 +243,16 @@ monitor_reactors_write(struct file *file, const char __user *user_buf, 


	retval = -EINVAL;



	list_for_each_entry(rdef, &rv_reactors_list, list) {

		if (strcmp(ptr, rdef->reactor->name) != 0)

	list_for_each_entry(reactor, &rv_reactors_list, list) {

		if (strcmp(ptr, reactor->name) != 0)

			continue;



		if (rdef == get_reactor_rdef_by_name("nop"))

		if (strcmp(reactor->name, "nop"))

			enable = false;

		else

			enable = true;



		monitor_swap_reactors(mon, rdef, enable);

		monitor_swap_reactors(mon, reactor, enable);



		retval = count;

		break;
@@ -299,23 +299,16 @@ static const struct file_operations monitor_reactors_ops = { 


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) {

		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);

			return -EINVAL;

		}

	}



	r = kzalloc(sizeof(struct rv_reactor_def), GFP_KERNEL);

	if (!r)

		return -ENOMEM;



	r->reactor = reactor;

	r->counter = 0;



	list_add_tail(&r->list, &rv_reactors_list);

	list_add_tail(&reactor->list, &rv_reactors_list);



	return 0;

}
@@ -350,26 +343,19 @@ int rv_register_reactor(struct rv_reactor *reactor) 
 */

int rv_unregister_reactor(struct rv_reactor *reactor)

{

	struct rv_reactor_def *ptr, *next;

	int ret = 0;



	mutex_lock(&rv_interface_lock);



	list_for_each_entry_safe(ptr, next, &rv_reactors_list, list) {

		if (strcmp(reactor->name, ptr->reactor->name) == 0) {



			if (!ptr->counter) {

				list_del(&ptr->list);

	if (!reactor->counter) {

		list_del(&reactor->list);

	} else {

		printk(KERN_WARNING

		       "rv: the rv_reactor %s is in use by %d monitor(s)\n",

				       ptr->reactor->name, ptr->counter);

		       reactor->name, reactor->counter);

		printk(KERN_WARNING "rv: the rv_reactor %s cannot be removed\n",

				       ptr->reactor->name);

		       reactor->name);

		ret = -EBUSY;

				break;

			}

		}

	}



	mutex_unlock(&rv_interface_lock);
@@ -469,8 +455,8 @@ int reactor_populate_monitor(struct rv_monitor *mon) 
	/*

	 * Configure as the rv_nop reactor.

	 */

	mon->rdef = get_reactor_rdef_by_name("nop");

	mon->rdef->counter++;

	mon->reactor = get_reactor_rdef_by_name("nop");

	mon->reactor->counter++;

	mon->reacting = false;



	return 0;
@@ -483,8 +469,8 @@ int reactor_populate_monitor(struct rv_monitor *mon) 
void reactor_cleanup_monitor(struct rv_monitor *mon)

{

	lockdep_assert_held(&rv_interface_lock);

	mon->rdef->counter--;

	WARN_ON_ONCE(mon->rdef->counter < 0);

	mon->reactor->counter--;

	WARN_ON_ONCE(mon->reactor->counter < 0);

}



/*