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: <CAM9d7cgj4Wj+0ZFQ3XfG12kkp4ThE4RT1+g=A1aSOinCXM9w+Q@mail.gmail.com> Date: Mon, 4 Dec 2023 15:49:45 -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 16/50] perf maps: Add remove maps function to remove a map based on callback On Mon, Nov 27, 2023 at 2:10 PM Ian Rogers <irogers@...gle.com> wrote: > > Removing maps wasn't being done under the write lock. Similar to > maps__for_each_map, iterate the entries but in this case remove the > entry based on the result of the callback. If an entry was removed > then maps_by_name also needs updating, so add missed flush. > > In dso__load_kcore, the test of map to save would always be false with > REFCNT_CHECKING because of a missing RC_CHK_ACCESS. > > Signed-off-by: Ian Rogers <irogers@...gle.com> Acked-by: Namhyung Kim <namhyung@...nel.org> A nitpick below, > --- [SNIP] > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 72f03b875478..30da8a405d11 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1239,13 +1239,23 @@ static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data) > return 0; > } > > +static bool remove_old_maps(struct map *map, void *data) > +{ > + const struct map *map_to_save = data; > + > + /* > + * We need to preserve eBPF maps even if they are covered by kcore, > + * because we need to access eBPF dso for source data. > + */ > + return RC_CHK_ACCESS(map) != RC_CHK_ACCESS(map_to_save) && !__map__is_bpf_prog(map); RC_CHK_EQUAL(map, map_to_save) ? Thanks, Namhyung > +} > + > static int dso__load_kcore(struct dso *dso, struct map *map, > const char *kallsyms_filename) > { > struct maps *kmaps = map__kmaps(map); > struct kcore_mapfn_data md; > struct map *replacement_map = NULL; > - struct map_rb_node *old_node, *next; > struct machine *machine; > bool is_64_bit; > int err, fd; > @@ -1292,17 +1302,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > } > > /* Remove old maps */ > - maps__for_each_entry_safe(kmaps, old_node, next) { > - struct map *old_map = old_node->map; > - > - /* > - * We need to preserve eBPF maps even if they are > - * covered by kcore, because we need to access > - * eBPF dso for source data. > - */ > - if (old_map != map && !__map__is_bpf_prog(old_map)) > - maps__remove(kmaps, old_map); > - } > + maps__remove_maps(kmaps, remove_old_maps, map); > machine->trampolines_mapped = false; > > /* Find the kernel map using the '_stext' symbol */ > -- > 2.43.0.rc1.413.gea7ed67945-goog >
Powered by blists - more mailing lists