On Mon, Oct 28, 2024 at 03:12:33AM -0400, Steven Rostedt wrote: > From: Steven Rostedt <[email protected]> > > The ftrace_lock is taken for most of the ftrace_graph_set_hash() function > throughout the end. Use guard to take the ftrace_lock to simplify the exit > paths. > > Signed-off-by: Steven Rostedt (Google) <[email protected]> > --- > kernel/trace/ftrace.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index c0fabd7da5b2..b4ef469f4fd2 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -6816,12 +6816,10 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char > *buffer) > > func_g.len = strlen(func_g.search); > > - mutex_lock(&ftrace_lock); > + guard(mutex)(&ftrace_lock); > > - if (unlikely(ftrace_disabled)) { > - mutex_unlock(&ftrace_lock); > + if (unlikely(ftrace_disabled)) > return -ENODEV; > - } > > do_for_each_ftrace_rec(pg, rec) { > > @@ -6837,7 +6835,7 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char > *buffer) > if (entry) > continue; > if (add_hash_entry(hash, rec->ip) == NULL) > - goto out; > + return 0; > } else { > if (entry) { > free_hash_entry(hash, entry); > @@ -6846,13 +6844,8 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char > *buffer) > } > } > } while_for_each_ftrace_rec(); > -out: > - mutex_unlock(&ftrace_lock); > > - if (fail) > - return -EINVAL; > - > - return 0; > + return fail ? -EINVAL : 0; > }
Isn't the fail case more a case of -ESRCH / -ENOENT rather than -EINVAL? Anyway, that's orthogonal, the patch preserves existing semantics and looks okay (as do the others fwiw).
