lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAM9d7chbysNaH++LQgeskZ_LRAF7KuErexapDTWRf-zsgPOq1g@mail.gmail.com> Date: Mon, 4 Dec 2023 15:59:09 -0800 From: Namhyung Kim <namhyung@...nel.org> To: Ian Rogers <irogers@...gle.com> Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, Nick Terrell <terrelln@...com>, Kan Liang <kan.liang@...ux.intel.com>, Andi Kleen <ak@...ux.intel.com>, Kajol Jain <kjain@...ux.ibm.com>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, Huacai Chen <chenhuacai@...nel.org>, Masami Hiramatsu <mhiramat@...nel.org>, Vincent Whitchurch <vincent.whitchurch@...s.com>, "Steinar H. Gunderson" <sesse@...gle.com>, Liam Howlett <liam.howlett@...cle.com>, Miguel Ojeda <ojeda@...nel.org>, Colin Ian King <colin.i.king@...il.com>, Dmitrii Dolgov <9erthalion6@...il.com>, Yang Jihong <yangjihong1@...wei.com>, Ming Wang <wangming01@...ngson.cn>, James Clark <james.clark@....com>, K Prateek Nayak <kprateek.nayak@....com>, Sean Christopherson <seanjc@...gle.com>, Leo Yan <leo.yan@...aro.org>, Ravi Bangoria <ravi.bangoria@....com>, German Gomez <german.gomez@....com>, Changbin Du <changbin.du@...wei.com>, Paolo Bonzini <pbonzini@...hat.com>, Li Dong <lidong@...o.com>, Sandipan Das <sandipan.das@....com>, liuwenyu <liuwenyu7@...wei.com>, linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, Guilherme Amadio <amadio@...too.org> Subject: Re: [PATCH v5 18/50] perf maps: Refactor maps__fixup_overlappings On Mon, Nov 27, 2023 at 2:10 PM Ian Rogers <irogers@...gle.com> wrote: > > Rename to maps__fixup_overlap_and_insert as the given mapping is > always inserted. Factor out first_ending_after as a utility > function. Minor variable name changes. Switch to using debug_file() > rather than passing a debug FILE*. > > Signed-off-by: Ian Rogers <irogers@...gle.com> > --- > tools/perf/util/maps.c | 62 ++++++++++++++++++++++++---------------- > tools/perf/util/maps.h | 2 +- > tools/perf/util/thread.c | 3 +- > 3 files changed, 39 insertions(+), 28 deletions(-) > > diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c > index f13fd3a9686b..40df08dd9bf3 100644 > --- a/tools/perf/util/maps.c > +++ b/tools/perf/util/maps.c > @@ -334,20 +334,16 @@ size_t maps__fprintf(struct maps *maps, FILE *fp) > return args.printed; > } > > -int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > +/* > + * Find first map where end > new->start. s/new/map/. > + * Same as find_vma() in kernel. > + */ > +static struct rb_node *first_ending_after(struct maps *maps, const struct map *map) > { > struct rb_root *root; > struct rb_node *next, *first; > - int err = 0; > - > - down_write(maps__lock(maps)); > > root = maps__entries(maps); > - > - /* > - * Find first map where end > map->start. > - * Same as find_vma() in kernel. > - */ > next = root->rb_node; > first = NULL; > while (next) { > @@ -361,8 +357,22 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > } else > next = next->rb_right; > } > + return first; > +} > + > +/* > + * Adds new to maps, if new overlaps existing entries then the existing maps are > + * adjusted or removed so that new fits without overlapping any entries. > + */ > +int maps__fixup_overlap_and_insert(struct maps *maps, struct map *new) Do we really need this rename (map -> new)? It seems just to create unnecessary diffs. Also I'd like to avoid 'new' as it's a keyword in C++ although we don't compile with C++ compilers. > +{ > + > + struct rb_node *next; > + int err = 0; Maybe you can add this line or let the caller pass it to reduce the diff. FILE *fp = debug_file(); Thanks, Namhyung > + > + down_write(maps__lock(maps)); > > - next = first; > + next = first_ending_after(maps, new); > while (next && !err) { > struct map_rb_node *pos = rb_entry(next, struct map_rb_node, rb_node); > next = rb_next(&pos->rb_node); > @@ -371,27 +381,27 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > * Stop if current map starts after map->end. > * Maps are ordered by start: next will not overlap for sure. > */ > - if (map__start(pos->map) >= map__end(map)) > + if (map__start(pos->map) >= map__end(new)) > break; > > if (verbose >= 2) { > > if (use_browser) { > pr_debug("overlapping maps in %s (disable tui for more info)\n", > - map__dso(map)->name); > + map__dso(new)->name); > } else { > - fputs("overlapping maps:\n", fp); > - map__fprintf(map, fp); > - map__fprintf(pos->map, fp); > + pr_debug("overlapping maps:\n"); > + map__fprintf(new, debug_file()); > + map__fprintf(pos->map, debug_file()); > } > } > > - rb_erase_init(&pos->rb_node, root); > + rb_erase_init(&pos->rb_node, maps__entries(maps)); > /* > * Now check if we need to create new maps for areas not > * overlapped by the new map: > */ > - if (map__start(map) > map__start(pos->map)) { > + if (map__start(new) > map__start(pos->map)) { > struct map *before = map__clone(pos->map); > > if (before == NULL) { > @@ -399,7 +409,7 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > goto put_map; > } > > - map__set_end(before, map__start(map)); > + map__set_end(before, map__start(new)); > err = __maps__insert(maps, before); > if (err) { > map__put(before); > @@ -407,11 +417,11 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > } > > if (verbose >= 2 && !use_browser) > - map__fprintf(before, fp); > + map__fprintf(before, debug_file()); > map__put(before); > } > > - if (map__end(map) < map__end(pos->map)) { > + if (map__end(new) < map__end(pos->map)) { > struct map *after = map__clone(pos->map); > > if (after == NULL) { > @@ -419,23 +429,25 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) > goto put_map; > } > > - map__set_start(after, map__end(map)); > - map__add_pgoff(after, map__end(map) - map__start(pos->map)); > - assert(map__map_ip(pos->map, map__end(map)) == > - map__map_ip(after, map__end(map))); > + map__set_start(after, map__end(new)); > + map__add_pgoff(after, map__end(new) - map__start(pos->map)); > + assert(map__map_ip(pos->map, map__end(new)) == > + map__map_ip(after, map__end(new))); > err = __maps__insert(maps, after); > if (err) { > map__put(after); > goto put_map; > } > if (verbose >= 2 && !use_browser) > - map__fprintf(after, fp); > + map__fprintf(after, debug_file()); > map__put(after); > } > put_map: > map__put(pos->map); > free(pos); > } > + /* Add the map. */ > + err = __maps__insert(maps, new); > up_write(maps__lock(maps)); > return err; > } > diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h > index b94ad5c8fea7..62e94d443c02 100644 > --- a/tools/perf/util/maps.h > +++ b/tools/perf/util/maps.h > @@ -133,7 +133,7 @@ struct addr_map_symbol; > > int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams); > > -int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp); > +int maps__fixup_overlap_and_insert(struct maps *maps, struct map *new); > > struct map *maps__find_by_name(struct maps *maps, const char *name); > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c > index b6986a81aa6d..3d47b5c5528b 100644 > --- a/tools/perf/util/thread.c > +++ b/tools/perf/util/thread.c > @@ -345,8 +345,7 @@ int thread__insert_map(struct thread *thread, struct map *map) > if (ret) > return ret; > > - maps__fixup_overlappings(thread__maps(thread), map, stderr); > - return maps__insert(thread__maps(thread), map); > + return maps__fixup_overlap_and_insert(thread__maps(thread), map); > } > > struct thread__prepare_access_maps_cb_args { > -- > 2.43.0.rc1.413.gea7ed67945-goog >
Powered by blists - more mailing lists