On Mon, Apr 22, 2024 at 03:09:31PM +0200, Vincenzo Mezzela wrote:It does indeed, I'll fix this.
Introduce the __free attribute for scope-based resource management.Keep it alphabetical. Also since <linux/of.h> does define kfree for
Resources allocated with __free are automatically released at the end of
the scope. This enhancement aims to mitigate memory management issues
associated with forgetting to release resources by utilizing __free
instead of of_node_put().
The declaration of the device_node used within the do-while loops is
moved directly within the loop so that the resource is automatically
freed at the end of each iteration.
Suggested-by: Julia Lawall <julia.lawall@xxxxxxxx>
Signed-off-by: Vincenzo Mezzela <vincenzo.mezzela@xxxxxxxxx>
---
changes in v2:
- check loop exit condition within the loop
- add cleanup.h header
drivers/base/arch_topology.c | 150 +++++++++++++++++------------------
1 file changed, 73 insertions(+), 77 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 024b78a0cfc1..c9c4af55953e 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -20,6 +20,7 @@
#include <linux/rcupdate.h>
#include <linux/sched.h>
#include <linux/units.h>
+#include <linux/cleanup.h>
of_node_get(), may not be needed strictly. Sorry for not noticing those
details earlier. I am fine either way, it is good to keep it IMO.
#define CREATE_TRACE_POINTSDid you run checkpatch.pl on this ? It should have complained here and 3 other
#include <trace/events/thermal_pressure.h>
@@ -513,10 +514,10 @@ core_initcall(free_raw_capacity);
*/
static int __init get_cpu_for_node(struct device_node *node)
{
- struct device_node *cpu_node;
int cpu;
- cpu_node = of_parse_phandle(node, "cpu", 0);
+ struct device_node *cpu_node __free(device_node) =
+ of_parse_phandle(node, "cpu", 0);
if (!cpu_node)
return -1;
@@ -527,7 +528,6 @@ static int __init get_cpu_for_node(struct device_node *node)
pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
cpu_node, cpumask_pr_args(cpu_possible_mask));
- of_node_put(cpu_node);
return cpu;
}
@@ -538,28 +538,27 @@ static int __init parse_core(struct device_node *core, int package_id,
bool leaf = true;
int i = 0;
int cpu;
- struct device_node *t;
- do {
+ for(;;) {
places below.
- if (leaf) {Missing alignment here.
- ret = parse_core(c, package_id, cluster_id,
- core_id++);
- } else {
- pr_err("%pOF: Non-leaf cluster with core %s\n",
- cluster, name);
- ret = -EINVAL;
- }
+ has_cores = true;
- of_node_put(c);
- if (ret != 0)
- return ret;
+ if (depth == 0) {
+ pr_err("%pOF: cpu-map children should be clusters\n", c);
+ return -EINVAL;
+ }
+
+ if (leaf) {
+ ret = parse_core(c, package_id, cluster_id, core_id++);
+ } else {
+ pr_err("%pOF: Non-leaf cluster with core %s\n",
+ cluster, name);
--
Regards,
Sudeep