mirror of
				https://github.com/torvalds/linux.git
				synced 2025-10-31 08:38:45 +02:00 
			
		
		
		
	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: 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:
		
							parent
							
								
									24cbfe18d5
								
							
						
					
					
						commit
						3d3c376118
					
				
					 3 changed files with 43 additions and 63 deletions
				
			
		|  | @ -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 | ||||
|  |  | |||
|  | @ -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); | ||||
|  |  | |||
|  | @ -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); | ||||
| } | ||||
| 
 | ||||
| /*
 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Nam Cao
						Nam Cao