Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1341684 > unrolled thread
| Started by | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| First post | 2016-02-24 10:10 +0100 |
| Last post | 2016-03-03 02:20 +0100 |
| Articles | 7 — 2 participants |
Back to article view | Back to linux.kernel
[RFC 0/4] perf kvm: Guest Symbol Resolution for powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-02-24 10:10 +0100
[RFC 2/4] perf kvm: Introduce evsel as argument to perf_event__preprocess_sample Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-02-24 10:10 +0100
[RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-02-24 10:20 +0100
Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-03-02 15:30 +0100
Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-03-02 16:50 +0100
Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-03-02 17:30 +0100
Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-03-03 02:20 +0100
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-02-24 10:10 +0100 |
| Subject | [RFC 0/4] perf kvm: Guest Symbol Resolution for powerpc |
| Message-ID | <r5DCi-2ih-19@gated-at.bofh.it> |
Design of [patch v2] Guest Symbol Resolution is focused on enabling
perf kvm {record|report} on powerpc. Here is the link for the same:
thread.gmane.org/gmane.linux.kernel/2132409
As per the point raised by acme, this design does not enable cross
arch reporting functionality. i.e. record on powerpc and report on
!powerpc.
This patch aims to enable cross arch reporting functionality along with
enabling perf kvm {record|report} on powerpc. Note that basic principle
of enabling perf kvm {record|report} on powerpc using tracepoint
kvm_hv:kvm_guest_exit has not been changed.
Major change between [patch v2] and this [RFC] patch is, I've moved
'perf kvm report' related and ppc specific functionality from
tool/perf/arch/powerpc/ to generic tool/perf/ code. This is required
because perf binary needs ppc specific code even if it's compiled on
!ppc to enable cross arch reporting.
I need suggestion specifically on patch 3 (Enable 'report' on powerpc)
which contains arch specific code in generic area. Right now I've added
code in util/evsel.c. But please let me know if there's any better way
to do this.
This patch is to get suggestions on approach so I've tagged it as RFC
and not following the patch version series.
Ravi Bangoria (4):
perf kvm: Enable 'record' on powerpc
perf kvm: Introduce evsel as argument to perf_event__preprocess_sample
perf kvm: Enable 'report' on powerpc
perf kvm: Fix output fields instead of 'trace' for perf kvm report on
powerpc
tools/perf/arch/powerpc/util/Build | 1 +
tools/perf/arch/powerpc/util/kvm.c | 18 +++++++++
tools/perf/builtin-annotate.c | 3 +-
tools/perf/builtin-diff.c | 3 +-
tools/perf/builtin-mem.c | 10 +++--
tools/perf/builtin-report.c | 8 +++-
tools/perf/builtin-script.c | 3 +-
tools/perf/builtin-timechart.c | 8 ++--
tools/perf/builtin-top.c | 3 +-
tools/perf/tests/hists_cumulate.c | 2 +-
tools/perf/tests/hists_filter.c | 2 +-
tools/perf/tests/hists_link.c | 4 +-
tools/perf/tests/hists_output.c | 2 +-
tools/perf/util/event.c | 8 ++--
tools/perf/util/event.h | 3 +-
tools/perf/util/evlist.c | 9 +++++
tools/perf/util/evlist.h | 1 +
tools/perf/util/evsel.c | 77 ++++++++++++++++++++++++++++++++++++++
tools/perf/util/evsel.h | 7 ++++
tools/perf/util/session.c | 7 ++--
tools/perf/util/util.c | 5 +++
tools/perf/util/util.h | 1 +
22 files changed, 161 insertions(+), 24 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/kvm.c
--
2.1.4
[toc] | [next] | [standalone]
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-02-24 10:10 +0100 |
| Subject | [RFC 2/4] perf kvm: Introduce evsel as argument to perf_event__preprocess_sample |
| Message-ID | <r5DCi-2ih-23@gated-at.bofh.it> |
| In reply to | #1341684 |
This patch changes prototype of perf_event__preprocess_sample() with
additional argument evsel added at the end.
This change is required because perf_event__preprocess_sample()
function will use evsel to determine cpumode of samples for powerpc
architecture.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
tools/perf/builtin-annotate.c | 3 ++-
tools/perf/builtin-diff.c | 3 ++-
tools/perf/builtin-mem.c | 10 ++++++----
tools/perf/builtin-report.c | 3 ++-
tools/perf/builtin-script.c | 3 ++-
tools/perf/builtin-timechart.c | 8 +++++---
tools/perf/builtin-top.c | 3 ++-
tools/perf/tests/hists_cumulate.c | 2 +-
tools/perf/tests/hists_filter.c | 2 +-
tools/perf/tests/hists_link.c | 4 ++--
tools/perf/tests/hists_output.c | 2 +-
tools/perf/util/event.c | 3 ++-
tools/perf/util/event.h | 3 ++-
13 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index cfe3663..da330ae 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -94,7 +94,8 @@ static int process_sample_event(struct perf_tool *tool,
struct addr_location al;
int ret = 0;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
pr_warning("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 36ccc2b..d2a27fe 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -330,7 +330,8 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,
struct hists *hists = evsel__hists(evsel);
int ret = -1;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
pr_warning("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index b3f8a89..a7c01fe 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -118,13 +118,15 @@ static int
dump_raw_samples(struct perf_tool *tool,
union perf_event *event,
struct perf_sample *sample,
- struct machine *machine)
+ struct machine *machine,
+ struct perf_evsel *evsel)
{
struct perf_mem *mem = container_of(tool, struct perf_mem, tool);
struct addr_location al;
const char *fmt;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
fprintf(stderr, "problem processing %d event, skipping it.\n",
event->header.type);
return -1;
@@ -168,10 +170,10 @@ out_put:
static int process_sample_event(struct perf_tool *tool,
union perf_event *event,
struct perf_sample *sample,
- struct perf_evsel *evsel __maybe_unused,
+ struct perf_evsel *evsel,
struct machine *machine)
{
- return dump_raw_samples(tool, event, sample, machine);
+ return dump_raw_samples(tool, event, sample, machine, evsel);
}
static int report_raw_events(struct perf_mem *mem)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 760e886..31ec4ba 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -154,7 +154,8 @@ static int process_sample_event(struct perf_tool *tool,
};
int ret = 0;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
pr_debug("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index f4caf48..792868e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -804,7 +804,8 @@ static int process_sample_event(struct perf_tool *tool,
return 0;
}
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
pr_err("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index bd7a775..a177396 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -470,7 +470,8 @@ static void sched_switch(struct timechart *tchart, int cpu, u64 timestamp,
static const char *cat_backtrace(union perf_event *event,
struct perf_sample *sample,
- struct machine *machine)
+ struct machine *machine,
+ struct perf_evsel *evsel)
{
struct addr_location al;
unsigned int i;
@@ -489,7 +490,8 @@ static const char *cat_backtrace(union perf_event *event,
if (!chain)
goto exit;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0) {
fprintf(stderr, "problem processing %d event, skipping it.\n",
event->header.type);
goto exit;
@@ -569,7 +571,7 @@ static int process_sample_event(struct perf_tool *tool,
if (evsel->handler != NULL) {
tracepoint_handler f = evsel->handler;
return f(tchart, evsel, sample,
- cat_backtrace(event, sample, machine));
+ cat_backtrace(event, sample, machine, evsel));
}
return 0;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a75de39..79ec8e1 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -728,7 +728,8 @@ static void perf_event__process_sample(struct perf_tool *tool,
if (event->header.misc & PERF_RECORD_MISC_EXACT_IP)
top->exact_samples++;
- if (perf_event__preprocess_sample(event, machine, &al, sample) < 0)
+ if (perf_event__preprocess_sample(event, machine, &al,
+ sample, evsel) < 0)
return;
if (!top->kptr_restrict_warned &&
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index ecf136c..d5cdd41 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -103,7 +103,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
sample.callchain = (struct ip_callchain *)fake_callchains[i];
if (perf_event__preprocess_sample(&event, machine, &al,
- &sample) < 0)
+ &sample, evsel) < 0)
goto out;
if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH,
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 34b945a..2bede64 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -81,7 +81,7 @@ static int add_hist_entries(struct perf_evlist *evlist,
sample.ip = fake_samples[i].ip;
if (perf_event__preprocess_sample(&event, machine, &al,
- &sample) < 0)
+ &sample, evsel) < 0)
goto out;
al.socket = fake_samples[i].socket;
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 64b257d..7b8c590 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -86,7 +86,7 @@ static int add_hist_entries(struct perf_evlist *evlist, struct machine *machine)
sample.tid = fake_common_samples[k].pid;
sample.ip = fake_common_samples[k].ip;
if (perf_event__preprocess_sample(&event, machine, &al,
- &sample) < 0)
+ &sample, evsel) < 0)
goto out;
he = __hists__add_entry(hists, &al, NULL,
@@ -112,7 +112,7 @@ static int add_hist_entries(struct perf_evlist *evlist, struct machine *machine)
sample.tid = fake_samples[i][k].pid;
sample.ip = fake_samples[i][k].ip;
if (perf_event__preprocess_sample(&event, machine, &al,
- &sample) < 0)
+ &sample, evsel) < 0)
goto out;
he = __hists__add_entry(hists, &al, NULL,
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index 23cce67..fba8996 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -69,7 +69,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
sample.ip = fake_samples[i].ip;
if (perf_event__preprocess_sample(&event, machine, &al,
- &sample) < 0)
+ &sample, evsel) < 0)
goto out;
if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH,
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 7bad5c3..bc0a3f0 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1298,7 +1298,8 @@ void thread__find_addr_location(struct thread *thread,
int perf_event__preprocess_sample(const union perf_event *event,
struct machine *machine,
struct addr_location *al,
- struct perf_sample *sample)
+ struct perf_sample *sample,
+ struct perf_evsel *evsel __maybe_unused)
{
u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
struct thread *thread = machine__findnew_thread(machine, sample->pid,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index b7ffb7e..fb8489b 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -600,7 +600,8 @@ struct addr_location;
int perf_event__preprocess_sample(const union perf_event *event,
struct machine *machine,
struct addr_location *al,
- struct perf_sample *sample);
+ struct perf_sample *sample,
+ struct perf_evsel *evsel);
void addr_location__put(struct addr_location *al);
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-02-24 10:20 +0100 |
| Subject | [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc |
| Message-ID | <r5DLY-2n7-19@gated-at.bofh.it> |
| In reply to | #1341684 |
commit d49dadea7862 ("perf tools: Make 'trace' or 'trace_fields' sort key
default for tracepoint events") makes 'trace' sort key as a default
while displaying report for tracepoint.
Because tracepoint(kvm_hv:kvm_guest_exit) is used as a default event,
perf kvm report will display output as a list of tracepoint hits and
not with a normal report columns.
This patch will replace 'trace' field with 'overhead,comm,dso,sym' while
displaying perf kvm report of powerpc.
Before applying patch:
$ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 181K of event 'kvm_hv:kvm_guest_exit'
# Event count (approx.): 181061
#
# Overhead Trace output
# ........ .................................................................................
#
0.02% VCPU 8: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
0.00% VCPU 0: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
0.00% VCPU 8: trap=HV_DECREMENTER pc=0x10005c7c msr=0x800000000280f032, ceded=0
0.00% VCPU 8: trap=HV_DECREMENTER pc=0x1001ef14 msr=0x800000000280f032, ceded=0
0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff83398830 msr=0x800000000280f032, ceded=0
0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a6fe4 msr=0x800000000280f032, ceded=0
0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a7a64 msr=0x800000000280f032, ceded=0
After applying patch:
$ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 181K of event 'kvm_hv:kvm_guest_exit'
# Event count (approx.): 181061
#
# Overhead Command Shared Object Symbol
# ........ ....... ....................... ..............................
#
0.02% :57276 [guest.kernel.kallsyms] [g] .plpar_hcall_norets
0.00% :57274 [guest.kernel.kallsyms] [g] .plpar_hcall_norets
0.00% :57276 [guest.kernel.kallsyms] [g] .__copy_tofrom_user_power7
0.00% :57276 [guest.kernel.kallsyms] [g] ._atomic_dec_and_lock
0.00% :57276 [guest.kernel.kallsyms] [g] ._raw_spin_lock
0.00% :57276 [guest.kernel.kallsyms] [g] ._switch
0.00% :57276 [guest.kernel.kallsyms] [g] .bio_add_page
0.00% :57276 [guest.kernel.kallsyms] [g] .kmem_cache_alloc
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
tools/perf/builtin-report.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 31ec4ba..5d96882 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -930,6 +930,11 @@ repeat:
else
use_browser = 0;
+ if (!field_order &&
+ is_perf_data_reorded_on_ppc(session->evlist) &&
+ perf_guest_only())
+ field_order = "overhead,comm,dso,sym";
+
if (setup_sorting(session->evlist) < 0) {
if (sort_order)
parse_options_usage(report_usage, options, "s", 1);
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-03-02 15:30 +0100 |
| Subject | Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc |
| Message-ID | <r8fWP-pB-47@gated-at.bofh.it> |
| In reply to | #1341695 |
Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
> commit d49dadea7862 ("perf tools: Make 'trace' or 'trace_fields' sort key
> default for tracepoint events") makes 'trace' sort key as a default
> while displaying report for tracepoint.
>
> Because tracepoint(kvm_hv:kvm_guest_exit) is used as a default event,
> perf kvm report will display output as a list of tracepoint hits and
> not with a normal report columns.
>
> This patch will replace 'trace' field with 'overhead,comm,dso,sym' while
> displaying perf kvm report of powerpc.
>
> Before applying patch:
>
> $ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 181K of event 'kvm_hv:kvm_guest_exit'
> # Event count (approx.): 181061
> #
> # Overhead Trace output
> # ........ .................................................................................
> #
> 0.02% VCPU 8: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
> 0.00% VCPU 0: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
> 0.00% VCPU 8: trap=HV_DECREMENTER pc=0x10005c7c msr=0x800000000280f032, ceded=0
> 0.00% VCPU 8: trap=HV_DECREMENTER pc=0x1001ef14 msr=0x800000000280f032, ceded=0
> 0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff83398830 msr=0x800000000280f032, ceded=0
> 0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a6fe4 msr=0x800000000280f032, ceded=0
> 0.00% VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a7a64 msr=0x800000000280f032, ceded=0
>
> After applying patch:
>
> $ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 181K of event 'kvm_hv:kvm_guest_exit'
> # Event count (approx.): 181061
> #
> # Overhead Command Shared Object Symbol
> # ........ ....... ....................... ..............................
> #
> 0.02% :57276 [guest.kernel.kallsyms] [g] .plpar_hcall_norets
> 0.00% :57274 [guest.kernel.kallsyms] [g] .plpar_hcall_norets
> 0.00% :57276 [guest.kernel.kallsyms] [g] .__copy_tofrom_user_power7
> 0.00% :57276 [guest.kernel.kallsyms] [g] ._atomic_dec_and_lock
> 0.00% :57276 [guest.kernel.kallsyms] [g] ._raw_spin_lock
> 0.00% :57276 [guest.kernel.kallsyms] [g] ._switch
> 0.00% :57276 [guest.kernel.kallsyms] [g] .bio_add_page
> 0.00% :57276 [guest.kernel.kallsyms] [g] .kmem_cache_alloc
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> tools/perf/builtin-report.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 31ec4ba..5d96882 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -930,6 +930,11 @@ repeat:
> else
> use_browser = 0;
>
> + if (!field_order &&
> + is_perf_data_reorded_on_ppc(session->evlist) &&
> + perf_guest_only())
> + field_order = "overhead,comm,dso,sym";
> +
Can you please do it as:
__weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
{
}
This way we don't see any arch specific stuff in the tool, also I
haven't seen any doc update, are you sure nothing needs to be added to
tools/perf/Documentaton/ for any of these patches?
I think this needs to be documented further, probably in
tools/perf/design.txt too?
> if (setup_sorting(session->evlist) < 0) {
> if (sort_order)
> parse_options_usage(report_usage, options, "s", 1);
> --
> 2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-03-02 16:50 +0100 |
| Subject | Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc |
| Message-ID | <r8hce-1c4-21@gated-at.bofh.it> |
| In reply to | #1348086 |
Thanks Arnaldo,
Please find my comments.
On Wednesday 02 March 2016 07:55 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
>> use_browser = 0;
>>
>> + if (!field_order &&
>> + is_perf_data_reorded_on_ppc(session->evlist) &&
>> + perf_guest_only())
>> + field_order = "overhead,comm,dso,sym";
>> +
> Can you please do it as:
>
> __weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
> {
> }
So you mean like this - Just implement only weak function and move code
into it?
ie. No strong implementation at this point of time.
Like,
__weak void arch__override_field_order(struct perf_evlist *evlist, const
char **f_order)
{
if (!field_order &&
is_perf_data_reorded_on_ppc(session->evlist) &&
perf_guest_only())
*field_order = "overhead,comm,dso,sym";
}
Then I can do that.
But if you are proposing to implement a strong function and move this code
into in, then we won't be able to enable cross arch reporting.
>
> This way we don't see any arch specific stuff in the tool, also I
> haven't seen any doc update, are you sure nothing needs to be added to
> tools/perf/Documentaton/ for any of these patches?
>
> I think this needs to be documented further, probably in
> tools/perf/design.txt too?
Yes, I'll do this in next version.
Regards,
Ravi
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-03-02 17:30 +0100 |
| Subject | Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc |
| Message-ID | <r8hOX-1I2-35@gated-at.bofh.it> |
| In reply to | #1348190 |
Em Wed, Mar 02, 2016 at 09:16:48PM +0530, Ravi Bangoria escreveu:
> Thanks Arnaldo,
>
> Please find my comments.
>
> On Wednesday 02 March 2016 07:55 PM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
> >> use_browser = 0;
> >>+ if (!field_order &&
> >>+ is_perf_data_reorded_on_ppc(session->evlist) &&
> >>+ perf_guest_only())
> >>+ field_order = "overhead,comm,dso,sym";
> >>+
> >Can you please do it as:
> >
> >__weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
> >{
> >}
>
> So you mean like this - Just implement only weak function and move code into
> it?
> ie. No strong implementation at this point of time.
>
> Like,
>
> __weak void arch__override_field_order(struct perf_evlist *evlist, const
> char **f_order)
> {
> if (!field_order &&
> is_perf_data_reorded_on_ppc(session->evlist) &&
Oh, I see, ugh, when running on x86_64 we wouldn't use this, so we need
to have per arch default field orders, now I have to recall why is it
that we need this per-arch field order :-\
- Arnaldo
> perf_guest_only())
> *field_order = "overhead,comm,dso,sym";
> }
>
> Then I can do that.
>
> But if you are proposing to implement a strong function and move this code
> into in, then we won't be able to enable cross arch reporting.
>
> >
> >This way we don't see any arch specific stuff in the tool, also I
> >haven't seen any doc update, are you sure nothing needs to be added to
> >tools/perf/Documentaton/ for any of these patches?
> >
> >I think this needs to be documented further, probably in
> >tools/perf/design.txt too?
>
> Yes, I'll do this in next version.
>
> Regards,
> Ravi
[toc] | [prev] | [next] | [standalone]
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-03-03 02:20 +0100 |
| Subject | Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc |
| Message-ID | <r8q5Q-80V-5@gated-at.bofh.it> |
| In reply to | #1348302 |
Thanks acme,
On Wednesday 02 March 2016 09:52 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 02, 2016 at 09:16:48PM +0530, Ravi Bangoria escreveu:
>> Thanks Arnaldo,
>>
>> Please find my comments.
>>
>> On Wednesday 02 March 2016 07:55 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
>>>> use_browser = 0;
>>>> + if (!field_order &&
>>>> + is_perf_data_reorded_on_ppc(session->evlist) &&
>>>> + perf_guest_only())
>>>> + field_order = "overhead,comm,dso,sym";
>>>> +
>>> Can you please do it as:
>>>
>>> __weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
>>> {
>>> }
>> So you mean like this - Just implement only weak function and move code into
>> it?
>> ie. No strong implementation at this point of time.
>>
>> Like,
>>
>> __weak void arch__override_field_order(struct perf_evlist *evlist, const
>> char **f_order)
>> {
>> if (!field_order &&
>> is_perf_data_reorded_on_ppc(session->evlist) &&
> Oh, I see, ugh, when running on x86_64 we wouldn't use this, so we need
> to have per arch default field orders, now I have to recall why is it
> that we need this per-arch field order :-\
Sorry, I'm little bit confused. We need arch specific functionality present
on all arch to make cross arch reporting possible.
for example, record perf.data on ppc and report on x86, we need
ppc specific function present in perf binary compiled on x86.
Please let me know if I understood it wrong.
Regads,
Ravi
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web