Messages in this thread | | | From | Namhyung Kim <> | Date | Thu, 25 May 2023 23:00:54 -0700 | Subject | Re: [PATCH v1 01/16] perf header: Make nodes dynamic in write_mem_topology |
| |
On Thu, May 25, 2023 at 7:39 PM Ian Rogers <irogers@google.com> wrote: > > On Thu, May 25, 2023 at 12:15 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Thu, May 25, 2023 at 12:12 AM Ian Rogers <irogers@google.com> wrote: > > > > > > Avoid a large static array, dynamically allocate the nodes avoiding a > > > hard coded limited as well. > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > --- > > > tools/perf/util/header.c | 33 +++++++++++++++++++++------------ > > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > > > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > > > index 2dde3ca20de5..80593ed8c79b 100644 > > > --- a/tools/perf/util/header.c > > > +++ b/tools/perf/util/header.c > > > @@ -24,6 +24,7 @@ > > > #include <bpf/libbpf.h> > > > #endif > > > #include <perf/cpumap.h> > > > +#include <tools/libc_compat.h> // reallocarray > > > > > > #include "dso.h" > > > #include "evlist.h" > > > @@ -1396,13 +1397,14 @@ static int memory_node__sort(const void *a, const void *b) > > > return na->node - nb->node; > > > } > > > > > > -static int build_mem_topology(struct memory_node *nodes, u64 size, u64 *cntp) > > > +static int build_mem_topology(struct memory_node **nodesp, u64 *cntp) > > > { > > > char path[PATH_MAX]; > > > struct dirent *ent; > > > DIR *dir; > > > - u64 cnt = 0; > > > int ret = 0; > > > + size_t cnt = 0, size = 0; > > > + struct memory_node *nodes = NULL; > > > > > > scnprintf(path, PATH_MAX, "%s/devices/system/node/", > > > sysfs__mountpoint()); > > > @@ -1426,16 +1428,24 @@ static int build_mem_topology(struct memory_node *nodes, u64 size, u64 *cntp) > > > if (r != 1) > > > continue; > > > > > > - if (WARN_ONCE(cnt >= size, > > > - "failed to write MEM_TOPOLOGY, way too many nodes\n")) { > > > - closedir(dir); > > > - return -1; > > > - } > > > + if (cnt >= size) { > > > + struct memory_node *new_nodes = > > > + reallocarray(nodes, cnt + 4, sizeof(*nodes)); > > > > > > + if (!new_nodes) { > > > + pr_err("Failed to write MEM_TOPOLOGY, size %zd nodes\n", size); > > > + free(nodes); > > > + closedir(dir); > > > + return -ENOMEM; > > > + } > > > + nodes = new_nodes; > > > + size += 4; > > > + } > > > ret = memory_node__read(&nodes[cnt++], idx); > > > > I think you need to handle error cases here. > > > > Thanks, > > Namhyung > > Not sure I follow. The reallocarray tests for failure, frees nodes and > returns -ENOMEM on error.
Right, but I think it would leak the nodes when the memory_node__read() fails.
Thanks, Namhyung
| |