Messages in this thread | | | From | Namhyung Kim <> | Date | Wed, 27 Jan 2021 16:37:51 +0900 | Subject | Re: [PATCH v3 1/2] perf tools: add 'perf irq' to measure the hardware interrupts |
| |
Hello,
On Sat, Jan 16, 2021 at 10:20 AM Bixuan Cui <cuibixuan@huawei.com> wrote: > > Add 'perf irq' to trace/measure the hardware interrupts. > > Now three functions are provided: > 1. 'perf irq record <command>' to record the irq handler events. > 2. 'perf irq script' to see a detailed trace of the workload that > was recorded. > 3. 'perf irq report' to calculate the time consumed by each > hardware interrupt processing function. > > Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> > --- > tools/perf/Build | 1 + > tools/perf/builtin-irq.c | 283 +++++++++++++++++++++++++++++++++++++++ > tools/perf/builtin.h | 1 + > tools/perf/perf.c | 1 + > 4 files changed, 286 insertions(+) > create mode 100644 tools/perf/builtin-irq.c > > diff --git a/tools/perf/Build b/tools/perf/Build > index 5f392dbb88fc..d52a1e1d6d8a 100644 > --- a/tools/perf/Build > +++ b/tools/perf/Build > @@ -24,6 +24,7 @@ perf-y += builtin-mem.o > perf-y += builtin-data.o > perf-y += builtin-version.o > perf-y += builtin-c2c.o > +perf-y += builtin-irq.o > > perf-$(CONFIG_TRACE) += builtin-trace.o > perf-$(CONFIG_LIBELF) += builtin-probe.o > diff --git a/tools/perf/builtin-irq.c b/tools/perf/builtin-irq.c > new file mode 100644 > index 000000000000..25ba0669a875 > --- /dev/null > +++ b/tools/perf/builtin-irq.c [SNIP] > + > +#define IRQ_NAME_LEN 20 > +#define MAX_CPUS 4096 > + > +static const char *cpu_list; > +static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS); > + > +struct perf_irq;
Seems unnecessary.
> + > +struct perf_irq { > + struct perf_tool tool; > + bool force; > + > + u32 irq_entry_irq; > + char irq_name[IRQ_NAME_LEN]; > + u32 cpu; > + u64 irq_entry_time; > + u32 irq_entry_pid; > + u32 irq_exit_irq; > + u64 irq_exit_time; > + u32 irq_exit_pid; > +}; > + > +typedef int (*irq_handler)(struct perf_tool *tool, > + union perf_event *event, > + struct evsel *evsel, > + struct perf_sample *sample, > + struct machine *machine);
You don't need to pass all the arguments if unused.
> + > +static int perf_report_process_sample(struct perf_tool *tool, > + union perf_event *event, > + struct perf_sample *sample, > + struct evsel *evsel, > + struct machine *machine) > +{ > + int err = 0; > + > + if (evsel->handler != NULL) { > + irq_handler f = evsel->handler; > + err = f(tool, event, evsel, sample, machine); > + } > + > + return err; > +} > + > +static void output_report(struct perf_irq *irq) > +{ > + int ret, i; > + char irq_entry_time[30], irq_exit_time[30], irq_diff[30]; > + > + /* The entry and exit of the hardware irq function > + * exist at the same time. Check it by irq and pid. > + */ > + if (irq->irq_entry_pid != irq->irq_exit_pid || > + irq->irq_entry_irq != irq->irq_exit_irq) > + return;
Is there only a single instance of the perf_irq here? Then I don't think this is correct and you should keep pairs of irq entry/exit per cpu. Otherwise overlapped irqs from different cpus will be discarded (wrongly).
> + > + timestamp__scnprintf_usec(irq->irq_entry_time, > + irq_entry_time, sizeof(irq_entry_time)); > + timestamp__scnprintf_usec(irq->irq_exit_time, > + irq_exit_time, sizeof(irq_exit_time)); > + timestamp__scnprintf_usec(irq->irq_exit_time - irq->irq_entry_time, > + irq_diff, sizeof(irq_diff)); > + > + ret = printf(" %s ", irq->irq_name); > + for (i = 0; i < IRQ_NAME_LEN - ret; i++) > + printf(" "); > + > + printf("| [%04d] | %13s s | %16s s | %16s s\n", > + irq->cpu, irq_diff, irq_entry_time, irq_exit_time); > +} > + > +static int report_irq_handler_entry_event(struct perf_tool *tool, > + union perf_event *event __maybe_unused, > + struct evsel *evsel, > + struct perf_sample *sample, > + struct machine *machine __maybe_unused) > +{ > + int err = 0; > + struct perf_irq *irq = container_of(tool, struct perf_irq, tool); > + > + const char *name = evsel__strval(evsel, sample, "name"); > + > + irq->irq_entry_pid = evsel__intval(evsel, sample, "pid"); > + irq->irq_entry_irq = evsel__intval(evsel, sample, "irq"); > + irq->irq_entry_time = sample->time; > + strncpy(irq->irq_name, name, IRQ_NAME_LEN);
Note that strncpy doesn't guarantee the NUL-termination. You'd better do it by yourself just in case.
Thanks, Namhyung
> + > + return err; > +} > + > +static int report_irq_handler_exit_event(struct perf_tool *tool, > + union perf_event *event __maybe_unused, > + struct evsel *evsel, > + struct perf_sample *sample, > + struct machine *machine __maybe_unused) > +{ > + int err = 0; > + struct perf_irq *irq = container_of(tool, struct perf_irq, tool); > + > + irq->irq_exit_pid = evsel__intval(evsel, sample, "pid"); > + irq->irq_exit_irq = evsel__intval(evsel, sample, "irq"); > + irq->irq_exit_time = sample->time; > + irq->cpu = sample->cpu; > + > + if (cpu_list && !test_bit(sample->cpu, cpu_bitmap)) > + return err; > + > + output_report(irq); > + > + return err; > +} > +
| |