Skip to content

Commit

Permalink
ftrace: Switch ftrace.c code over to use guard()
Browse files Browse the repository at this point in the history
There are a few functions in ftrace.c that have "goto out" or equivalent
on error in order to release locks that were taken. This can be error
prone or just simply make the code more complex.

Switch every location that ends with unlocking a mutex on error over to
using the guard(mutex)() infrastructure to let the compiler worry about
releasing locks. This makes the code easier to read and understand.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: https://lore.kernel.org/20241223184941.718001540@goodmis.org
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
  • Loading branch information
Steven Rostedt committed Dec 24, 2024
1 parent 77e53cb commit 1d95fd9
Showing 1 changed file with 34 additions and 63 deletions.
97 changes: 34 additions & 63 deletions kernel/trace/ftrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -536,24 +536,21 @@ static int function_stat_show(struct seq_file *m, void *v)
{
struct ftrace_profile *rec = v;
char str[KSYM_SYMBOL_LEN];
int ret = 0;
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static struct trace_seq s;
unsigned long long avg;
unsigned long long stddev;
#endif
mutex_lock(&ftrace_profile_lock);
guard(mutex)(&ftrace_profile_lock);

/* we raced with function_profile_reset() */
if (unlikely(rec->counter == 0)) {
ret = -EBUSY;
goto out;
}
if (unlikely(rec->counter == 0))
return -EBUSY;

#ifdef CONFIG_FUNCTION_GRAPH_TRACER
avg = div64_ul(rec->time, rec->counter);
if (tracing_thresh && (avg < tracing_thresh))
goto out;
return 0;
#endif

kallsyms_lookup(rec->ip, NULL, NULL, NULL, str);
Expand Down Expand Up @@ -590,10 +587,8 @@ static int function_stat_show(struct seq_file *m, void *v)
trace_print_seq(m, &s);
#endif
seq_putc(m, '\n');
out:
mutex_unlock(&ftrace_profile_lock);

return ret;
return 0;
}

static void ftrace_profile_reset(struct ftrace_profile_stat *stat)
Expand Down Expand Up @@ -944,20 +939,16 @@ ftrace_profile_write(struct file *filp, const char __user *ubuf,

val = !!val;

mutex_lock(&ftrace_profile_lock);
guard(mutex)(&ftrace_profile_lock);
if (ftrace_profile_enabled ^ val) {
if (val) {
ret = ftrace_profile_init();
if (ret < 0) {
cnt = ret;
goto out;
}
if (ret < 0)
return ret;

ret = register_ftrace_profiler();
if (ret < 0) {
cnt = ret;
goto out;
}
if (ret < 0)
return ret;
ftrace_profile_enabled = 1;
} else {
ftrace_profile_enabled = 0;
Expand All @@ -968,8 +959,6 @@ ftrace_profile_write(struct file *filp, const char __user *ubuf,
unregister_ftrace_profiler();
}
}
out:
mutex_unlock(&ftrace_profile_lock);

*ppos += cnt;

Expand Down Expand Up @@ -5610,20 +5599,15 @@ static DEFINE_MUTEX(ftrace_cmd_mutex);
__init int register_ftrace_command(struct ftrace_func_command *cmd)
{
struct ftrace_func_command *p;
int ret = 0;

mutex_lock(&ftrace_cmd_mutex);
guard(mutex)(&ftrace_cmd_mutex);
list_for_each_entry(p, &ftrace_commands, list) {
if (strcmp(cmd->name, p->name) == 0) {
ret = -EBUSY;
goto out_unlock;
}
if (strcmp(cmd->name, p->name) == 0)
return -EBUSY;
}
list_add(&cmd->list, &ftrace_commands);
out_unlock:
mutex_unlock(&ftrace_cmd_mutex);

return ret;
return 0;
}

/*
Expand All @@ -5633,20 +5617,17 @@ __init int register_ftrace_command(struct ftrace_func_command *cmd)
__init int unregister_ftrace_command(struct ftrace_func_command *cmd)
{
struct ftrace_func_command *p, *n;
int ret = -ENODEV;

mutex_lock(&ftrace_cmd_mutex);
guard(mutex)(&ftrace_cmd_mutex);

list_for_each_entry_safe(p, n, &ftrace_commands, list) {
if (strcmp(cmd->name, p->name) == 0) {
ret = 0;
list_del_init(&p->list);
goto out_unlock;
return 0;
}
}
out_unlock:
mutex_unlock(&ftrace_cmd_mutex);

return ret;
return -ENODEV;
}

static int ftrace_process_regex(struct ftrace_iterator *iter,
Expand All @@ -5656,7 +5637,7 @@ static int ftrace_process_regex(struct ftrace_iterator *iter,
struct trace_array *tr = iter->ops->private;
char *func, *command, *next = buff;
struct ftrace_func_command *p;
int ret = -EINVAL;
int ret;

func = strsep(&next, ":");

Expand All @@ -5673,17 +5654,14 @@ static int ftrace_process_regex(struct ftrace_iterator *iter,

command = strsep(&next, ":");

mutex_lock(&ftrace_cmd_mutex);
guard(mutex)(&ftrace_cmd_mutex);

list_for_each_entry(p, &ftrace_commands, list) {
if (strcmp(p->name, command) == 0) {
ret = p->func(tr, hash, func, command, next, enable);
goto out_unlock;
}
if (strcmp(p->name, command) == 0)
return p->func(tr, hash, func, command, next, enable);
}
out_unlock:
mutex_unlock(&ftrace_cmd_mutex);

return ret;
return -EINVAL;
}

static ssize_t
Expand Down Expand Up @@ -8280,7 +8258,7 @@ pid_write(struct file *filp, const char __user *ubuf,
if (!cnt)
return 0;

mutex_lock(&ftrace_lock);
guard(mutex)(&ftrace_lock);

switch (type) {
case TRACE_PIDS:
Expand All @@ -8296,14 +8274,13 @@ pid_write(struct file *filp, const char __user *ubuf,
lockdep_is_held(&ftrace_lock));
break;
default:
ret = -EINVAL;
WARN_ON_ONCE(1);
goto out;
return -EINVAL;
}

ret = trace_pid_write(filtered_pids, &pid_list, ubuf, cnt);
if (ret < 0)
goto out;
return ret;

switch (type) {
case TRACE_PIDS:
Expand Down Expand Up @@ -8332,11 +8309,8 @@ pid_write(struct file *filp, const char __user *ubuf,

ftrace_update_pid_func();
ftrace_startup_all(0);
out:
mutex_unlock(&ftrace_lock);

if (ret > 0)
*ppos += ret;
*ppos += ret;

return ret;
}
Expand Down Expand Up @@ -8739,17 +8713,17 @@ static int
ftrace_enable_sysctl(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
int ret = -ENODEV;
int ret;

mutex_lock(&ftrace_lock);
guard(mutex)(&ftrace_lock);

if (unlikely(ftrace_disabled))
goto out;
return -ENODEV;

ret = proc_dointvec(table, write, buffer, lenp, ppos);

if (ret || !write || (last_ftrace_enabled == !!ftrace_enabled))
goto out;
return ret;

if (ftrace_enabled) {

Expand All @@ -8763,8 +8737,7 @@ ftrace_enable_sysctl(const struct ctl_table *table, int write,
} else {
if (is_permanent_ops_registered()) {
ftrace_enabled = true;
ret = -EBUSY;
goto out;
return -EBUSY;
}

/* stopping ftrace calls (just send to ftrace_stub) */
Expand All @@ -8774,9 +8747,7 @@ ftrace_enable_sysctl(const struct ctl_table *table, int write,
}

last_ftrace_enabled = !!ftrace_enabled;
out:
mutex_unlock(&ftrace_lock);
return ret;
return 0;
}

static struct ctl_table ftrace_sysctls[] = {
Expand Down

0 comments on commit 1d95fd9

Please sign in to comment.