Messages in this thread Patch in this message | | | From | Vincenzo Mezzela <> | Subject | [PATCH v2] drivers: use __free attribute instead of of_node_put() | Date | Mon, 22 Apr 2024 15:09:31 +0200 |
| |
Introduce the __free attribute for scope-based resource management. 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@inria.fr> Signed-off-by: Vincenzo Mezzela <vincenzo.mezzela@gmail.com> --- 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> #define CREATE_TRACE_POINTS #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(;;) { snprintf(name, sizeof(name), "thread%d", i); - t = of_get_child_by_name(core, name); - if (t) { - leaf = false; - cpu = get_cpu_for_node(t); - if (cpu >= 0) { - cpu_topology[cpu].package_id = package_id; - cpu_topology[cpu].cluster_id = cluster_id; - cpu_topology[cpu].core_id = core_id; - cpu_topology[cpu].thread_id = i; - } else if (cpu != -ENODEV) { - pr_err("%pOF: Can't get CPU for thread\n", t); - of_node_put(t); - return -EINVAL; - } - of_node_put(t); + struct device_node *t __free(device_node) = + of_get_child_by_name(core, name); + if (!t) + break; + + leaf = false; + cpu = get_cpu_for_node(t); + if (cpu >= 0) { + cpu_topology[cpu].package_id = package_id; + cpu_topology[cpu].cluster_id = cluster_id; + cpu_topology[cpu].core_id = core_id; + cpu_topology[cpu].thread_id = i; + } else if (cpu != -ENODEV) { + pr_err("%pOF: Can't get CPU for thread\n", t); + return -EINVAL; } i++; - } while (t); + } cpu = get_cpu_for_node(core); if (cpu >= 0) { @@ -586,7 +585,6 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, char name[20]; bool leaf = true; bool has_cores = false; - struct device_node *c; int core_id = 0; int i, ret; @@ -596,51 +594,51 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, * scheduler with a flat list of them. */ i = 0; - do { + for(;;) { snprintf(name, sizeof(name), "cluster%d", i); - c = of_get_child_by_name(cluster, name); - if (c) { - leaf = false; - ret = parse_cluster(c, package_id, i, depth + 1); - if (depth > 0) - pr_warn("Topology for clusters of clusters not yet supported\n"); - of_node_put(c); - if (ret != 0) - return ret; - } + struct device_node *c __free(device_node) = + of_get_child_by_name(cluster, name); + if (!c) + break; + + leaf = false; + ret = parse_cluster(c, package_id, i, depth + 1); + if (depth > 0) + pr_warn("Topology for clusters of clusters not yet supported\n"); + if (ret != 0) + return ret; i++; - } while (c); + } /* Now check for cores */ i = 0; - do { + for(;;) { snprintf(name, sizeof(name), "core%d", i); - c = of_get_child_by_name(cluster, name); - if (c) { - has_cores = true; - - if (depth == 0) { - pr_err("%pOF: cpu-map children should be clusters\n", - c); - of_node_put(c); - return -EINVAL; - } + struct device_node *c __free(device_node) = + of_get_child_by_name(cluster, name); + if (!c) + break; - 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); - 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); + ret = -EINVAL; } + + if (ret != 0) + return ret; + i++; - } while (c); + } if (leaf && !has_cores) pr_warn("%pOF: empty cluster\n", cluster); @@ -651,22 +649,23 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, static int __init parse_socket(struct device_node *socket) { char name[20]; - struct device_node *c; bool has_socket = false; int package_id = 0, ret; - do { + for(;;) { snprintf(name, sizeof(name), "socket%d", package_id); - c = of_get_child_by_name(socket, name); - if (c) { - has_socket = true; - ret = parse_cluster(c, package_id, -1, 0); - of_node_put(c); - if (ret != 0) - return ret; - } + struct device_node *c __free(device_node) = + of_get_child_by_name(socket, name); + if (!c) + break; + + has_socket = true; + ret = parse_cluster(c, package_id, -1, 0); + if (ret != 0) + return ret; + package_id++; - } while (c); + } if (!has_socket) ret = parse_cluster(socket, 0, -1, 0); @@ -676,11 +675,11 @@ static int __init parse_socket(struct device_node *socket) static int __init parse_dt_topology(void) { - struct device_node *cn, *map; int ret = 0; int cpu; - cn = of_find_node_by_path("/cpus"); + struct device_node *cn __free(device_node) = + of_find_node_by_path("/cpus"); if (!cn) { pr_err("No CPU information found in DT\n"); return 0; @@ -690,13 +689,14 @@ static int __init parse_dt_topology(void) * When topology is provided cpu-map is essentially a root * cluster with restricted subnodes. */ - map = of_get_child_by_name(cn, "cpu-map"); + struct device_node *map __free(device_node) = + of_get_child_by_name(cn, "cpu-map"); if (!map) - goto out; + return ret; ret = parse_socket(map); if (ret != 0) - goto out_map; + return ret; topology_normalize_cpu_scale(); @@ -710,10 +710,6 @@ static int __init parse_dt_topology(void) break; } -out_map: - of_node_put(map); -out: - of_node_put(cn); return ret; } #endif -- 2.34.1
| |