Hello, These are some missing pieces that I found during the code-reading. Thanks, Namhyung Namhyung Kim (3): perf tools: Protect reading thread's namespace perf tools: Add missing swap ops for namespace events perf top: Enable --namespaces option tools/perf/Documentation/perf-top.txt | 5 +++++ tools/perf/builtin-top.c | 5 +++++ tools/perf/util/session.c | 21 +++++++++++++++++++++ tools/perf/util/thread.c | 15 +++++++++++++-- 4 files changed, 44 insertions(+), 2 deletions(-) -- 2.21.0.1020.gf2820cf01a-goog
It seems that the current code lacks holding the namespace lock in thread__namespaces(). Otherwise it can see inconsistent results. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/thread.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 403045a2bbea..b413ba5b9835 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -133,7 +133,7 @@ void thread__put(struct thread *thread) } } -struct namespaces *thread__namespaces(const struct thread *thread) +static struct namespaces *__thread__namespaces(const struct thread *thread) { if (list_empty(&thread->namespaces_list)) return NULL; @@ -141,10 +141,21 @@ struct namespaces *thread__namespaces(const struct thread *thread) return list_first_entry(&thread->namespaces_list, struct namespaces, list); } +struct namespaces *thread__namespaces(const struct thread *thread) +{ + struct namespaces *ns; + + down_read((struct rw_semaphore *)&thread->namespaces_lock); + ns = __thread__namespaces(thread); + up_read((struct rw_semaphore *)&thread->namespaces_lock); + + return ns; +} + static int __thread__set_namespaces(struct thread *thread, u64 timestamp, struct namespaces_event *event) { - struct namespaces *new, *curr = thread__namespaces(thread); + struct namespaces *new, *curr = __thread__namespaces(thread); new = namespaces__new(event); if (!new) -- 2.21.0.1020.gf2820cf01a-goog
In case it's recorded from other arch. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/session.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 2310a1752983..54cf163347f7 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -647,6 +647,26 @@ static void perf_event__throttle_swap(union perf_event *event, swap_sample_id_all(event, &event->throttle + 1); } +static void perf_event__namespaces_swap(union perf_event *event, + bool sample_id_all) +{ + u64 i; + + event->namespaces.pid = bswap_32(event->namespaces.pid); + event->namespaces.tid = bswap_32(event->namespaces.tid); + event->namespaces.nr_namespaces = bswap_64(event->namespaces.nr_namespaces); + + for (i = 0; i < event->namespaces.nr_namespaces; i++) { + struct perf_ns_link_info *ns = &event->namespaces.link_info[i]; + + ns->dev = bswap_64(ns->dev); + ns->ino = bswap_64(ns->ino); + } + + if (sample_id_all) + swap_sample_id_all(event, &event->namespaces.link_info[i]); +} + static u8 revbyte(u8 b) { int rev = (b >> 4) | ((b & 0xf) << 4); @@ -887,6 +907,7 @@ static perf_event__swap_op perf_event__swap_ops[] = { [PERF_RECORD_LOST_SAMPLES] = perf_event__all64_swap, [PERF_RECORD_SWITCH] = perf_event__switch_swap, [PERF_RECORD_SWITCH_CPU_WIDE] = perf_event__switch_swap, + [PERF_RECORD_NAMESPACES] = perf_event__namespaces_swap, [PERF_RECORD_HEADER_ATTR] = perf_event__hdr_attr_swap, [PERF_RECORD_HEADER_EVENT_TYPE] = perf_event__event_type_swap, [PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap, -- 2.21.0.1020.gf2820cf01a-goog
Since perf record already have the option, let's have it for perf top as well. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/Documentation/perf-top.txt | 5 +++++ tools/perf/builtin-top.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt index 44d89fb9c788..cfea87c6f38e 100644 --- a/tools/perf/Documentation/perf-top.txt +++ b/tools/perf/Documentation/perf-top.txt @@ -262,6 +262,11 @@ Default is to monitor all CPUS. The number of threads to run when synthesizing events for existing processes. By default, the number of threads equals to the number of online CPUs. +--namespaces:: + Record events of type PERF_RECORD_NAMESPACES and display it with the + 'cgroup_id' sort key. + + INTERACTIVE PROMPTING KEYS -------------------------- diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 3648ef576996..6651377fd762 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1208,6 +1208,9 @@ static int __cmd_top(struct perf_top *top) init_process_thread(top); + if (opts->record_namespaces) + top->tool.namespace_events = true; + ret = perf_event__synthesize_bpf_events(top->session, perf_event__process, &top->session->machines.host, &top->record_opts); @@ -1500,6 +1503,8 @@ int cmd_top(int argc, const char **argv) OPT_BOOLEAN(0, "force", &symbol_conf.force, "don't complain, do it"), OPT_UINTEGER(0, "num-thread-synthesize", &top.nr_threads_synthesize, "number of thread to run event synthesize"), + OPT_BOOLEAN(0, "namespaces", &opts->record_namespaces, + "Record namespaces events"), OPT_END() }; struct perf_evlist *sb_evlist = NULL; -- 2.21.0.1020.gf2820cf01a-goog
Em Wed, May 22, 2019 at 02:32:48PM +0900, Namhyung Kim escreveu: > It seems that the current code lacks holding the namespace lock in > thread__namespaces(). Otherwise it can see inconsistent results. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/thread.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c > index 403045a2bbea..b413ba5b9835 100644 > --- a/tools/perf/util/thread.c > +++ b/tools/perf/util/thread.c > @@ -133,7 +133,7 @@ void thread__put(struct thread *thread) > } > } > > -struct namespaces *thread__namespaces(const struct thread *thread) > +static struct namespaces *__thread__namespaces(const struct thread *thread) > { > if (list_empty(&thread->namespaces_list)) > return NULL; > @@ -141,10 +141,21 @@ struct namespaces *thread__namespaces(const struct thread *thread) > return list_first_entry(&thread->namespaces_list, struct namespaces, list); > } > > +struct namespaces *thread__namespaces(const struct thread *thread) > +{ > + struct namespaces *ns; > + > + down_read((struct rw_semaphore *)&thread->namespaces_lock); > + ns = __thread__namespaces(thread); > + up_read((struct rw_semaphore *)&thread->namespaces_lock); > + Humm, so we need to change thread__namespaces() to remove that const instead of throwing it away with that cast, right? - Arnaldo > + return ns; > +} > + > static int __thread__set_namespaces(struct thread *thread, u64 timestamp, > struct namespaces_event *event) > { > - struct namespaces *new, *curr = thread__namespaces(thread); > + struct namespaces *new, *curr = __thread__namespaces(thread); > > new = namespaces__new(event); > if (!new) > -- > 2.21.0.1020.gf2820cf01a-goog -- - Arnaldo
Em Wed, May 22, 2019 at 02:32:49PM +0900, Namhyung Kim escreveu: > In case it's recorded from other arch. Applied and added: Fixes: f3b3614a284d ("perf tools: Add PERF_RECORD_NAMESPACES to include namespaces related info") So that the stable guys can pick it. - Arnaldo > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/session.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index 2310a1752983..54cf163347f7 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -647,6 +647,26 @@ static void perf_event__throttle_swap(union perf_event *event, > swap_sample_id_all(event, &event->throttle + 1); > } > > +static void perf_event__namespaces_swap(union perf_event *event, > + bool sample_id_all) > +{ > + u64 i; > + > + event->namespaces.pid = bswap_32(event->namespaces.pid); > + event->namespaces.tid = bswap_32(event->namespaces.tid); > + event->namespaces.nr_namespaces = bswap_64(event->namespaces.nr_namespaces); > + > + for (i = 0; i < event->namespaces.nr_namespaces; i++) { > + struct perf_ns_link_info *ns = &event->namespaces.link_info[i]; > + > + ns->dev = bswap_64(ns->dev); > + ns->ino = bswap_64(ns->ino); > + } > + > + if (sample_id_all) > + swap_sample_id_all(event, &event->namespaces.link_info[i]); > +} > + > static u8 revbyte(u8 b) > { > int rev = (b >> 4) | ((b & 0xf) << 4); > @@ -887,6 +907,7 @@ static perf_event__swap_op perf_event__swap_ops[] = { > [PERF_RECORD_LOST_SAMPLES] = perf_event__all64_swap, > [PERF_RECORD_SWITCH] = perf_event__switch_swap, > [PERF_RECORD_SWITCH_CPU_WIDE] = perf_event__switch_swap, > + [PERF_RECORD_NAMESPACES] = perf_event__namespaces_swap, > [PERF_RECORD_HEADER_ATTR] = perf_event__hdr_attr_swap, > [PERF_RECORD_HEADER_EVENT_TYPE] = perf_event__event_type_swap, > [PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap, > -- > 2.21.0.1020.gf2820cf01a-goog -- - Arnaldo
Em Wed, May 22, 2019 at 02:32:50PM +0900, Namhyung Kim escreveu: > Since perf record already have the option, let's have it for perf top > as well. I'm applying, but I wonder if this shouldn't be the default... - Arnaldo > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/Documentation/perf-top.txt | 5 +++++ > tools/perf/builtin-top.c | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt > index 44d89fb9c788..cfea87c6f38e 100644 > --- a/tools/perf/Documentation/perf-top.txt > +++ b/tools/perf/Documentation/perf-top.txt > @@ -262,6 +262,11 @@ Default is to monitor all CPUS. > The number of threads to run when synthesizing events for existing processes. > By default, the number of threads equals to the number of online CPUs. > > +--namespaces:: > + Record events of type PERF_RECORD_NAMESPACES and display it with the > + 'cgroup_id' sort key. > + > + > INTERACTIVE PROMPTING KEYS > -------------------------- > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 3648ef576996..6651377fd762 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -1208,6 +1208,9 @@ static int __cmd_top(struct perf_top *top) > > init_process_thread(top); > > + if (opts->record_namespaces) > + top->tool.namespace_events = true; > + > ret = perf_event__synthesize_bpf_events(top->session, perf_event__process, > &top->session->machines.host, > &top->record_opts); > @@ -1500,6 +1503,8 @@ int cmd_top(int argc, const char **argv) > OPT_BOOLEAN(0, "force", &symbol_conf.force, "don't complain, do it"), > OPT_UINTEGER(0, "num-thread-synthesize", &top.nr_threads_synthesize, > "number of thread to run event synthesize"), > + OPT_BOOLEAN(0, "namespaces", &opts->record_namespaces, > + "Record namespaces events"), > OPT_END() > }; > struct perf_evlist *sb_evlist = NULL; > -- > 2.21.0.1020.gf2820cf01a-goog -- - Arnaldo
Hi Arnaldo,
On Wed, May 22, 2019 at 10:18:32AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 22, 2019 at 02:32:48PM +0900, Namhyung Kim escreveu:
> > It seems that the current code lacks holding the namespace lock in
> > thread__namespaces(). Otherwise it can see inconsistent results.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/thread.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > index 403045a2bbea..b413ba5b9835 100644
> > --- a/tools/perf/util/thread.c
> > +++ b/tools/perf/util/thread.c
> > @@ -133,7 +133,7 @@ void thread__put(struct thread *thread)
> > }
> > }
> >
> > -struct namespaces *thread__namespaces(const struct thread *thread)
> > +static struct namespaces *__thread__namespaces(const struct thread *thread)
> > {
> > if (list_empty(&thread->namespaces_list))
> > return NULL;
> > @@ -141,10 +141,21 @@ struct namespaces *thread__namespaces(const struct thread *thread)
> > return list_first_entry(&thread->namespaces_list, struct namespaces, list);
> > }
> >
> > +struct namespaces *thread__namespaces(const struct thread *thread)
> > +{
> > + struct namespaces *ns;
> > +
> > + down_read((struct rw_semaphore *)&thread->namespaces_lock);
> > + ns = __thread__namespaces(thread);
> > + up_read((struct rw_semaphore *)&thread->namespaces_lock);
> > +
>
> Humm, so we need to change thread__namespaces() to remove that const
> instead of throwing it away with that cast, right?
Yes, that's possible too. Note that thread__comm_str() has the same issue
as well.
Thanks,
Namhyung
On Wed, May 22, 2019 at 10:24:24AM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, May 22, 2019 at 02:32:50PM +0900, Namhyung Kim escreveu: > > Since perf record already have the option, let's have it for perf top > > as well. > > I'm applying, but I wonder if this shouldn't be the default... Not sure, it'll only add a bit of overhead to task and/or namespace creation. Thanks, Namhyung > > - Arnaldo > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/Documentation/perf-top.txt | 5 +++++ > > tools/perf/builtin-top.c | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt > > index 44d89fb9c788..cfea87c6f38e 100644 > > --- a/tools/perf/Documentation/perf-top.txt > > +++ b/tools/perf/Documentation/perf-top.txt > > @@ -262,6 +262,11 @@ Default is to monitor all CPUS. > > The number of threads to run when synthesizing events for existing processes. > > By default, the number of threads equals to the number of online CPUs. > > > > +--namespaces:: > > + Record events of type PERF_RECORD_NAMESPACES and display it with the > > + 'cgroup_id' sort key. > > + > > + > > INTERACTIVE PROMPTING KEYS > > -------------------------- > > > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > > index 3648ef576996..6651377fd762 100644 > > --- a/tools/perf/builtin-top.c > > +++ b/tools/perf/builtin-top.c > > @@ -1208,6 +1208,9 @@ static int __cmd_top(struct perf_top *top) > > > > init_process_thread(top); > > > > + if (opts->record_namespaces) > > + top->tool.namespace_events = true; > > + > > ret = perf_event__synthesize_bpf_events(top->session, perf_event__process, > > &top->session->machines.host, > > &top->record_opts); > > @@ -1500,6 +1503,8 @@ int cmd_top(int argc, const char **argv) > > OPT_BOOLEAN(0, "force", &symbol_conf.force, "don't complain, do it"), > > OPT_UINTEGER(0, "num-thread-synthesize", &top.nr_threads_synthesize, > > "number of thread to run event synthesize"), > > + OPT_BOOLEAN(0, "namespaces", &opts->record_namespaces, > > + "Record namespaces events"), > > OPT_END() > > }; > > struct perf_evlist *sb_evlist = NULL; > > -- > > 2.21.0.1020.gf2820cf01a-goog > > -- > > - Arnaldo
The namespaces and comm fields of a thread are protected by rwsem and require write access for it. So it ended up using a cast to remove the const qualifier. Let's get rid of the const then. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/hist.c | 2 +- tools/perf/util/thread.c | 12 ++++++------ tools/perf/util/thread.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 7ace7a10054d..fb3271fd420c 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -2561,7 +2561,7 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh char unit; int printed; const struct dso *dso = hists->dso_filter; - const struct thread *thread = hists->thread_filter; + struct thread *thread = hists->thread_filter; int socket_id = hists->socket_filter; unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE]; u64 nr_events = hists->stats.total_period; diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index b413ba5b9835..aab7807d445f 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -141,13 +141,13 @@ static struct namespaces *__thread__namespaces(const struct thread *thread) return list_first_entry(&thread->namespaces_list, struct namespaces, list); } -struct namespaces *thread__namespaces(const struct thread *thread) +struct namespaces *thread__namespaces(struct thread *thread) { struct namespaces *ns; - down_read((struct rw_semaphore *)&thread->namespaces_lock); + down_read(&thread->namespaces_lock); ns = __thread__namespaces(thread); - up_read((struct rw_semaphore *)&thread->namespaces_lock); + up_read(&thread->namespaces_lock); return ns; } @@ -271,13 +271,13 @@ static const char *__thread__comm_str(const struct thread *thread) return comm__str(comm); } -const char *thread__comm_str(const struct thread *thread) +const char *thread__comm_str(struct thread *thread) { const char *str; - down_read((struct rw_semaphore *)&thread->comm_lock); + down_read(&thread->comm_lock); str = __thread__comm_str(thread); - up_read((struct rw_semaphore *)&thread->comm_lock); + up_read(&thread->comm_lock); return str; } diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index cf8375c017a0..e97ef6977eb9 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -76,7 +76,7 @@ static inline void thread__exited(struct thread *thread) thread->dead = true; } -struct namespaces *thread__namespaces(const struct thread *thread); +struct namespaces *thread__namespaces(struct thread *thread); int thread__set_namespaces(struct thread *thread, u64 timestamp, struct namespaces_event *event); @@ -93,7 +93,7 @@ int thread__set_comm_from_proc(struct thread *thread); int thread__comm_len(struct thread *thread); struct comm *thread__comm(const struct thread *thread); struct comm *thread__exec_comm(const struct thread *thread); -const char *thread__comm_str(const struct thread *thread); +const char *thread__comm_str(struct thread *thread); int thread__insert_map(struct thread *thread, struct map *map); int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone); size_t thread__fprintf(struct thread *thread, FILE *fp); -- 2.22.0.rc1.257.g3120a18244-goog
Em Mon, May 27, 2019 at 03:11:49PM +0900, Namhyung Kim escreveu: > The namespaces and comm fields of a thread are protected by rwsem and > require write access for it. So it ended up using a cast to remove > the const qualifier. Let's get rid of the const then. Thanks, applied 1/3 and 4/3. - Arnaldo > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/hist.c | 2 +- > tools/perf/util/thread.c | 12 ++++++------ > tools/perf/util/thread.h | 4 ++-- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index 7ace7a10054d..fb3271fd420c 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -2561,7 +2561,7 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh > char unit; > int printed; > const struct dso *dso = hists->dso_filter; > - const struct thread *thread = hists->thread_filter; > + struct thread *thread = hists->thread_filter; > int socket_id = hists->socket_filter; > unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE]; > u64 nr_events = hists->stats.total_period; > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c > index b413ba5b9835..aab7807d445f 100644 > --- a/tools/perf/util/thread.c > +++ b/tools/perf/util/thread.c > @@ -141,13 +141,13 @@ static struct namespaces *__thread__namespaces(const struct thread *thread) > return list_first_entry(&thread->namespaces_list, struct namespaces, list); > } > > -struct namespaces *thread__namespaces(const struct thread *thread) > +struct namespaces *thread__namespaces(struct thread *thread) > { > struct namespaces *ns; > > - down_read((struct rw_semaphore *)&thread->namespaces_lock); > + down_read(&thread->namespaces_lock); > ns = __thread__namespaces(thread); > - up_read((struct rw_semaphore *)&thread->namespaces_lock); > + up_read(&thread->namespaces_lock); > > return ns; > } > @@ -271,13 +271,13 @@ static const char *__thread__comm_str(const struct thread *thread) > return comm__str(comm); > } > > -const char *thread__comm_str(const struct thread *thread) > +const char *thread__comm_str(struct thread *thread) > { > const char *str; > > - down_read((struct rw_semaphore *)&thread->comm_lock); > + down_read(&thread->comm_lock); > str = __thread__comm_str(thread); > - up_read((struct rw_semaphore *)&thread->comm_lock); > + up_read(&thread->comm_lock); > > return str; > } > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h > index cf8375c017a0..e97ef6977eb9 100644 > --- a/tools/perf/util/thread.h > +++ b/tools/perf/util/thread.h > @@ -76,7 +76,7 @@ static inline void thread__exited(struct thread *thread) > thread->dead = true; > } > > -struct namespaces *thread__namespaces(const struct thread *thread); > +struct namespaces *thread__namespaces(struct thread *thread); > int thread__set_namespaces(struct thread *thread, u64 timestamp, > struct namespaces_event *event); > > @@ -93,7 +93,7 @@ int thread__set_comm_from_proc(struct thread *thread); > int thread__comm_len(struct thread *thread); > struct comm *thread__comm(const struct thread *thread); > struct comm *thread__exec_comm(const struct thread *thread); > -const char *thread__comm_str(const struct thread *thread); > +const char *thread__comm_str(struct thread *thread); > int thread__insert_map(struct thread *thread, struct map *map); > int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone); > size_t thread__fprintf(struct thread *thread, FILE *fp); > -- > 2.22.0.rc1.257.g3120a18244-goog -- - Arnaldo
Commit-ID: 6584140ba9e6762dd7ec73795243289b914f31f9 Gitweb: https://git.kernel.org/tip/6584140ba9e6762dd7ec73795243289b914f31f9 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Wed, 22 May 2019 14:32:48 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 28 May 2019 09:52:23 -0300 perf namespace: Protect reading thread's namespace It seems that the current code lacks holding the namespace lock in thread__namespaces(). Otherwise it can see inconsistent results. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Krister Johansen <kjlx@templeofstupid.com> Link: http://lkml.kernel.org/r/20190522053250.207156-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/thread.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 403045a2bbea..b413ba5b9835 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -133,7 +133,7 @@ void thread__put(struct thread *thread) } } -struct namespaces *thread__namespaces(const struct thread *thread) +static struct namespaces *__thread__namespaces(const struct thread *thread) { if (list_empty(&thread->namespaces_list)) return NULL; @@ -141,10 +141,21 @@ struct namespaces *thread__namespaces(const struct thread *thread) return list_first_entry(&thread->namespaces_list, struct namespaces, list); } +struct namespaces *thread__namespaces(const struct thread *thread) +{ + struct namespaces *ns; + + down_read((struct rw_semaphore *)&thread->namespaces_lock); + ns = __thread__namespaces(thread); + up_read((struct rw_semaphore *)&thread->namespaces_lock); + + return ns; +} + static int __thread__set_namespaces(struct thread *thread, u64 timestamp, struct namespaces_event *event) { - struct namespaces *new, *curr = thread__namespaces(thread); + struct namespaces *new, *curr = __thread__namespaces(thread); new = namespaces__new(event); if (!new)
Commit-ID: acd244b84b80d53fa2cee98659b55d3f09b4f5a7 Gitweb: https://git.kernel.org/tip/acd244b84b80d53fa2cee98659b55d3f09b4f5a7 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Wed, 22 May 2019 14:32:49 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 28 May 2019 09:52:23 -0300 perf session: Add missing swap ops for namespace events In case it's recorded in a different arch. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com> <hbathini@linux.vnet.ibm.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Krister Johansen <kjlx@templeofstupid.com> Fixes: f3b3614a284d ("perf tools: Add PERF_RECORD_NAMESPACES to include namespaces related info") Link: http://lkml.kernel.org/r/20190522053250.207156-3-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/session.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 2310a1752983..54cf163347f7 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -647,6 +647,26 @@ static void perf_event__throttle_swap(union perf_event *event, swap_sample_id_all(event, &event->throttle + 1); } +static void perf_event__namespaces_swap(union perf_event *event, + bool sample_id_all) +{ + u64 i; + + event->namespaces.pid = bswap_32(event->namespaces.pid); + event->namespaces.tid = bswap_32(event->namespaces.tid); + event->namespaces.nr_namespaces = bswap_64(event->namespaces.nr_namespaces); + + for (i = 0; i < event->namespaces.nr_namespaces; i++) { + struct perf_ns_link_info *ns = &event->namespaces.link_info[i]; + + ns->dev = bswap_64(ns->dev); + ns->ino = bswap_64(ns->ino); + } + + if (sample_id_all) + swap_sample_id_all(event, &event->namespaces.link_info[i]); +} + static u8 revbyte(u8 b) { int rev = (b >> 4) | ((b & 0xf) << 4); @@ -887,6 +907,7 @@ static perf_event__swap_op perf_event__swap_ops[] = { [PERF_RECORD_LOST_SAMPLES] = perf_event__all64_swap, [PERF_RECORD_SWITCH] = perf_event__switch_swap, [PERF_RECORD_SWITCH_CPU_WIDE] = perf_event__switch_swap, + [PERF_RECORD_NAMESPACES] = perf_event__namespaces_swap, [PERF_RECORD_HEADER_ATTR] = perf_event__hdr_attr_swap, [PERF_RECORD_HEADER_EVENT_TYPE] = perf_event__event_type_swap, [PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap,
Commit-ID: a0c0a4ac021b017e385d0328541ccfebeef165fc Gitweb: https://git.kernel.org/tip/a0c0a4ac021b017e385d0328541ccfebeef165fc Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Wed, 22 May 2019 14:32:50 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 28 May 2019 18:37:43 -0300 perf top: Add --namespaces option Since 'perf record' already have this option, let's have it for 'perf top' as well. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Krister Johansen <kjlx@templeofstupid.com> Link: http://lkml.kernel.org/r/20190522053250.207156-4-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/Documentation/perf-top.txt | 5 +++++ tools/perf/builtin-top.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt index 44d89fb9c788..cfea87c6f38e 100644 --- a/tools/perf/Documentation/perf-top.txt +++ b/tools/perf/Documentation/perf-top.txt @@ -262,6 +262,11 @@ Default is to monitor all CPUS. The number of threads to run when synthesizing events for existing processes. By default, the number of threads equals to the number of online CPUs. +--namespaces:: + Record events of type PERF_RECORD_NAMESPACES and display it with the + 'cgroup_id' sort key. + + INTERACTIVE PROMPTING KEYS -------------------------- diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index fbbb0da43abb..31d78d874fc7 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1208,6 +1208,9 @@ static int __cmd_top(struct perf_top *top) init_process_thread(top); + if (opts->record_namespaces) + top->tool.namespace_events = true; + ret = perf_event__synthesize_bpf_events(top->session, perf_event__process, &top->session->machines.host, &top->record_opts); @@ -1500,6 +1503,8 @@ int cmd_top(int argc, const char **argv) OPT_BOOLEAN(0, "force", &symbol_conf.force, "don't complain, do it"), OPT_UINTEGER(0, "num-thread-synthesize", &top.nr_threads_synthesize, "number of thread to run event synthesize"), + OPT_BOOLEAN(0, "namespaces", &opts->record_namespaces, + "Record namespaces events"), OPT_END() }; struct perf_evlist *sb_evlist = NULL;
Commit-ID: 7cb10a08df98e643b87d4bc8422e50e9c43b5c60 Gitweb: https://git.kernel.org/tip/7cb10a08df98e643b87d4bc8422e50e9c43b5c60 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Mon, 27 May 2019 15:11:49 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 28 May 2019 18:37:43 -0300 perf tools: Remove const from thread read accessors The namespaces and comm fields of a thread are protected by rwsem and require write access for it. So it ended up using a cast to remove the const qualifier. Let's get rid of the const then. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Krister Johansen <kjlx@templeofstupid.com> Link: http://lkml.kernel.org/r/20190527061149.168640-1-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/hist.c | 2 +- tools/perf/util/thread.c | 12 ++++++------ tools/perf/util/thread.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 7ace7a10054d..fb3271fd420c 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -2561,7 +2561,7 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh char unit; int printed; const struct dso *dso = hists->dso_filter; - const struct thread *thread = hists->thread_filter; + struct thread *thread = hists->thread_filter; int socket_id = hists->socket_filter; unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE]; u64 nr_events = hists->stats.total_period; diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index b413ba5b9835..aab7807d445f 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -141,13 +141,13 @@ static struct namespaces *__thread__namespaces(const struct thread *thread) return list_first_entry(&thread->namespaces_list, struct namespaces, list); } -struct namespaces *thread__namespaces(const struct thread *thread) +struct namespaces *thread__namespaces(struct thread *thread) { struct namespaces *ns; - down_read((struct rw_semaphore *)&thread->namespaces_lock); + down_read(&thread->namespaces_lock); ns = __thread__namespaces(thread); - up_read((struct rw_semaphore *)&thread->namespaces_lock); + up_read(&thread->namespaces_lock); return ns; } @@ -271,13 +271,13 @@ static const char *__thread__comm_str(const struct thread *thread) return comm__str(comm); } -const char *thread__comm_str(const struct thread *thread) +const char *thread__comm_str(struct thread *thread) { const char *str; - down_read((struct rw_semaphore *)&thread->comm_lock); + down_read(&thread->comm_lock); str = __thread__comm_str(thread); - up_read((struct rw_semaphore *)&thread->comm_lock); + up_read(&thread->comm_lock); return str; } diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index cf8375c017a0..e97ef6977eb9 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -76,7 +76,7 @@ static inline void thread__exited(struct thread *thread) thread->dead = true; } -struct namespaces *thread__namespaces(const struct thread *thread); +struct namespaces *thread__namespaces(struct thread *thread); int thread__set_namespaces(struct thread *thread, u64 timestamp, struct namespaces_event *event); @@ -93,7 +93,7 @@ int thread__set_comm_from_proc(struct thread *thread); int thread__comm_len(struct thread *thread); struct comm *thread__comm(const struct thread *thread); struct comm *thread__exec_comm(const struct thread *thread); -const char *thread__comm_str(const struct thread *thread); +const char *thread__comm_str(struct thread *thread); int thread__insert_map(struct thread *thread, struct map *map); int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone); size_t thread__fprintf(struct thread *thread, FILE *fp);