Skip to contentSkip to conversation

tracing: Fix patches with goto out

PATCH New Steven Rostedt 1 participant 3 messages Dec 25, 2024

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

v2 cover tracing: Fix patches with goto out

In my patch series:

https://lore.kernel.org/linux-trace-kernel/[email protected]/

Of switching over to the guard logic. I missed a removal of a goto out in the osnoise, but fixed it in the stack trace patch.

Since the patches are supposed to be agnostic from each other, fix the osnoise patch, and remove the fix from the stack trace patch.

author maintainer
#1 lore raw

PATCH v2 1/2 tracing: Switch trace_osnoise.c code over to use guard() and __free()

The osnoise_hotplug_workfn() grabs two mutexes and cpu_read_lock(). It has various gotos to handle unlocking them. Switch them over to guard() and let the compiler worry about it.

The osnoise_cpus_read() has a temporary mask_str allocated and there's some gotos to make sure it gets freed on error paths. Switch that over to __free() to let the compiler worry about it.

---
Changes since v1: https://lore.kernel.org/[email protected]

- Fix the goto out error that was fixed in a later patch

 kernel/trace/trace_osnoise.c | 40 ++++++++++++------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)
kernel/trace/trace_osnoise.c +13 -27
index b9f96c77527d..b25c30b05dd0 100644
@@ -2083,26 +2083,21 @@ static void osnoise_hotplug_workfn(struct work_struct *dummy)
 {
 	unsigned int cpu = smp_processor_id();
 
-	mutex_lock(&trace_types_lock);
+	guard(mutex)(&trace_types_lock);
 
 	if (!osnoise_has_registered_instances())
-		goto out_unlock_trace;
+		return;
 
-	mutex_lock(&interface_lock);
-	cpus_read_lock();
+	guard(mutex)(&interface_lock);
+	guard(cpus_read_lock)();
 
 	if (!cpu_online(cpu))
-		goto out_unlock;
+		return;
+
 	if (!cpumask_test_cpu(cpu, &osnoise_cpumask))
-		goto out_unlock;
+		return;
 
 	start_kthread(cpu);
-
-out_unlock:
-	cpus_read_unlock();
-	mutex_unlock(&interface_lock);
-out_unlock_trace:
-	mutex_unlock(&trace_types_lock);
 }
 
 static DECLARE_WORK(osnoise_hotplug_work, osnoise_hotplug_workfn);
@@ -2300,31 +2295,22 @@ static ssize_t
 osnoise_cpus_read(struct file *filp, char __user *ubuf, size_t count,
 		  loff_t *ppos)
 {
-	char *mask_str;
+	char *mask_str __free(kfree) = NULL;
 	int len;
 
-	mutex_lock(&interface_lock);
+	guard(mutex)(&interface_lock);
 
 	len = snprintf(NULL, 0, "%*pbl\n", cpumask_pr_args(&osnoise_cpumask)) + 1;
 	mask_str = kmalloc(len, GFP_KERNEL);
-	if (!mask_str) {
-		count = -ENOMEM;
-		goto out_unlock;
-	}
+	if (!mask_str)
+		return -ENOMEM;
 
 	len = snprintf(mask_str, len, "%*pbl\n", cpumask_pr_args(&osnoise_cpumask));
-	if (len >= count) {
-		count = -EINVAL;
-		goto out_free;
-	}
+	if (len >= count)
+		return -EINVAL;
 
 	count = simple_read_from_buffer(ubuf, count, ppos, mask_str, len);
 
-out_free:
-	kfree(mask_str);
-out_unlock:
-	mutex_unlock(&interface_lock);
-
 	return count;
 }
 
Signature · 1 line
2.45.2
author maintainer
#2 lore raw

PATCH v2 2/2 tracing: Switch trace_stack.c code over to use guard()

In reply to Steven Rostedt

The function stack_trace_sysctl() uses a goto on the error path to jump to the mutex_unlock() code. Replace the logic to use guard() and let the compiler worry about it.

---
Changes since v1: https://lore.kernel.org/[email protected]

- Removed fix to osnoise tracer that did not belong in this patch

 kernel/trace/trace_stack.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/trace/trace_stack.c +2 -4
index 7f9572a37333..14c6f272c4d8 100644
@@ -520,20 +520,18 @@ stack_trace_sysctl(const struct ctl_table *table, int write, void *buffer,
 	int was_enabled;
 	int ret;
 
-	mutex_lock(&stack_sysctl_mutex);
+	guard(mutex)(&stack_sysctl_mutex);
 	was_enabled = !!stack_tracer_enabled;
 
 	ret = proc_dointvec(table, write, buffer, lenp, ppos);
 
 	if (ret || !write || (was_enabled == !!stack_tracer_enabled))
-		goto out;
+		return ret;
 
 	if (stack_tracer_enabled)
 		register_ftrace_function(&trace_ops);
 	else
 		unregister_ftrace_function(&trace_ops);
- out:
-	mutex_unlock(&stack_sysctl_mutex);
 	return ret;
 }
 
Signature · 1 line
2.45.2