Skip to content

Commit c81b995

Browse files
committed
Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull perf fixes from Thomas Gleixner: "A pile of perf updates: Kernel side: - Remove an incorrect warning in uprobe_init_insn() when insn_get_length() fails. The error return code is handled at the call site. - Move the inline keyword to the right place in the perf ringbuffer code to address a W=1 build warning. Tooling: perf stat: - Fix metric column header display alignment - Improve error messages for default attributes, providing better output for error in command line. - Add --interval-clear option, to provide a 'watch' like printing perf script: - Show hw-cache events too perf c2c: - Fix data dependency problem in layout of 'struct c2c_hist_entry' Core: - Do not blindly assume that 'struct perf_evsel' can be obtained via a straight forward container_of() as there are call sites which hand in a plain 'struct hist' which is not part of a container. - Fix error index in the PMU event parser, so that error messages can point to the problematic token" * 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: perf/core: Move the inline keyword at the beginning of the function declaration uprobes/x86: Remove incorrect WARN_ON() in uprobe_init_insn() perf script: Show hw-cache events perf c2c: Keep struct hist_entry at the end of struct c2c_hist_entry perf stat: Add event parsing error handling to add_default_attributes perf stat: Allow to specify specific metric column len perf stat: Fix metric column header display alignment perf stat: Use only color_fprintf call in print_metric_only perf stat: Add --interval-clear option perf tools: Fix error index for pmu event parser perf hists: Reimplement hists__has_callchains() perf hists browser gtk: Use hist_entry__has_callchains() perf hists: Make hist_entry__has_callchains() work with 'perf c2c' perf hists: Save the callchain_size in struct hist_entry
2 parents 2ce413e + 57d6a79 commit c81b995

File tree

11 files changed

+71
-37
lines changed

11 files changed

+71
-37
lines changed

arch/x86/kernel/uprobes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
293293
insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
294294
/* has the side-effect of processing the entire instruction */
295295
insn_get_length(insn);
296-
if (WARN_ON_ONCE(!insn_complete(insn)))
296+
if (!insn_complete(insn))
297297
return -ENOEXEC;
298298

299299
if (is_prefix_bad(insn))

kernel/events/ring_buffer.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ static void perf_output_put_handle(struct perf_output_handle *handle)
103103
preempt_enable();
104104
}
105105

106-
static bool __always_inline
106+
static __always_inline bool
107107
ring_buffer_has_space(unsigned long head, unsigned long tail,
108108
unsigned long data_size, unsigned int size,
109109
bool backward)
@@ -114,7 +114,7 @@ ring_buffer_has_space(unsigned long head, unsigned long tail,
114114
return CIRC_SPACE(tail, head, data_size) >= size;
115115
}
116116

117-
static int __always_inline
117+
static __always_inline int
118118
__perf_output_begin(struct perf_output_handle *handle,
119119
struct perf_event *event, unsigned int size,
120120
bool backward)
@@ -414,7 +414,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
414414
}
415415
EXPORT_SYMBOL_GPL(perf_aux_output_begin);
416416

417-
static bool __always_inline rb_need_aux_wakeup(struct ring_buffer *rb)
417+
static __always_inline bool rb_need_aux_wakeup(struct ring_buffer *rb)
418418
{
419419
if (rb->aux_overwrite)
420420
return false;

tools/perf/Documentation/perf-stat.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ Print count deltas for fixed number of times.
178178
This option should be used together with "-I" option.
179179
example: 'perf stat -I 1000 --interval-count 2 -e cycles -a'
180180

181+
--interval-clear::
182+
Clear the screen before next interval.
183+
181184
--timeout msecs::
182185
Stop the 'perf stat' session and print count deltas after N milliseconds (minimum: 10 ms).
183186
This option is not supported with the "-I" option.

tools/perf/builtin-c2c.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,16 @@ struct c2c_hist_entry {
5656

5757
struct compute_stats cstats;
5858

59+
unsigned long paddr;
60+
unsigned long paddr_cnt;
61+
bool paddr_zero;
62+
char *nodestr;
63+
5964
/*
6065
* must be at the end,
6166
* because of its callchain dynamic entry
6267
*/
6368
struct hist_entry he;
64-
65-
unsigned long paddr;
66-
unsigned long paddr_cnt;
67-
bool paddr_zero;
68-
char *nodestr;
6969
};
7070

7171
static char const *coalesce_default = "pid,iaddr";

tools/perf/builtin-script.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ static struct {
180180
PERF_OUTPUT_EVNAME | PERF_OUTPUT_TRACE
181181
},
182182

183+
[PERF_TYPE_HW_CACHE] = {
184+
.user_set = false,
185+
186+
.fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
187+
PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
188+
PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
189+
PERF_OUTPUT_SYM | PERF_OUTPUT_SYMOFFSET |
190+
PERF_OUTPUT_DSO | PERF_OUTPUT_PERIOD,
191+
192+
.invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
193+
},
194+
183195
[PERF_TYPE_RAW] = {
184196
.user_set = false,
185197

tools/perf/builtin-stat.c

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
#include "util/tool.h"
6666
#include "util/string2.h"
6767
#include "util/metricgroup.h"
68+
#include "util/top.h"
6869
#include "asm/bug.h"
6970

7071
#include <linux/time64.h>
@@ -144,6 +145,8 @@ static struct target target = {
144145

145146
typedef int (*aggr_get_id_t)(struct cpu_map *m, int cpu);
146147

148+
#define METRIC_ONLY_LEN 20
149+
147150
static int run_count = 1;
148151
static bool no_inherit = false;
149152
static volatile pid_t child_pid = -1;
@@ -173,13 +176,15 @@ static struct cpu_map *aggr_map;
173176
static aggr_get_id_t aggr_get_id;
174177
static bool append_file;
175178
static bool interval_count;
179+
static bool interval_clear;
176180
static const char *output_name;
177181
static int output_fd;
178182
static int print_free_counters_hint;
179183
static int print_mixed_hw_group_error;
180184
static u64 *walltime_run;
181185
static bool ru_display = false;
182186
static struct rusage ru_data;
187+
static unsigned int metric_only_len = METRIC_ONLY_LEN;
183188

184189
struct perf_stat {
185190
bool record;
@@ -967,8 +972,6 @@ static void print_metric_csv(void *ctx,
967972
fprintf(out, "%s%s%s%s", csv_sep, vals, csv_sep, unit);
968973
}
969974

970-
#define METRIC_ONLY_LEN 20
971-
972975
/* Filter out some columns that don't work well in metrics only mode */
973976

974977
static bool valid_only_metric(const char *unit)
@@ -999,22 +1002,20 @@ static void print_metric_only(void *ctx, const char *color, const char *fmt,
9991002
{
10001003
struct outstate *os = ctx;
10011004
FILE *out = os->fh;
1002-
int n;
1003-
char buf[1024];
1004-
unsigned mlen = METRIC_ONLY_LEN;
1005+
char buf[1024], str[1024];
1006+
unsigned mlen = metric_only_len;
10051007

10061008
if (!valid_only_metric(unit))
10071009
return;
10081010
unit = fixunit(buf, os->evsel, unit);
1009-
if (color)
1010-
n = color_fprintf(out, color, fmt, val);
1011-
else
1012-
n = fprintf(out, fmt, val);
1013-
if (n > METRIC_ONLY_LEN)
1014-
n = METRIC_ONLY_LEN;
10151011
if (mlen < strlen(unit))
10161012
mlen = strlen(unit) + 1;
1017-
fprintf(out, "%*s", mlen - n, "");
1013+
1014+
if (color)
1015+
mlen += strlen(color) + sizeof(PERF_COLOR_RESET) - 1;
1016+
1017+
color_snprintf(str, sizeof(str), color ?: "", fmt, val);
1018+
fprintf(out, "%*s ", mlen, str);
10181019
}
10191020

10201021
static void print_metric_only_csv(void *ctx, const char *color __maybe_unused,
@@ -1054,7 +1055,7 @@ static void print_metric_header(void *ctx, const char *color __maybe_unused,
10541055
if (csv_output)
10551056
fprintf(os->fh, "%s%s", unit, csv_sep);
10561057
else
1057-
fprintf(os->fh, "%-*s ", METRIC_ONLY_LEN, unit);
1058+
fprintf(os->fh, "%*s ", metric_only_len, unit);
10581059
}
10591060

10601061
static void nsec_printout(int id, int nr, struct perf_evsel *evsel, double avg)
@@ -1704,9 +1705,12 @@ static void print_interval(char *prefix, struct timespec *ts)
17041705
FILE *output = stat_config.output;
17051706
static int num_print_interval;
17061707

1708+
if (interval_clear)
1709+
puts(CONSOLE_CLEAR);
1710+
17071711
sprintf(prefix, "%6lu.%09lu%s", ts->tv_sec, ts->tv_nsec, csv_sep);
17081712

1709-
if (num_print_interval == 0 && !csv_output) {
1713+
if ((num_print_interval == 0 && !csv_output) || interval_clear) {
17101714
switch (stat_config.aggr_mode) {
17111715
case AGGR_SOCKET:
17121716
fprintf(output, "# time socket cpus");
@@ -1719,7 +1723,7 @@ static void print_interval(char *prefix, struct timespec *ts)
17191723
fprintf(output, " counts %*s events\n", unit_width, "unit");
17201724
break;
17211725
case AGGR_NONE:
1722-
fprintf(output, "# time CPU");
1726+
fprintf(output, "# time CPU ");
17231727
if (!metric_only)
17241728
fprintf(output, " counts %*s events\n", unit_width, "unit");
17251729
break;
@@ -1738,7 +1742,7 @@ static void print_interval(char *prefix, struct timespec *ts)
17381742
}
17391743
}
17401744

1741-
if (num_print_interval == 0 && metric_only)
1745+
if ((num_print_interval == 0 && metric_only) || interval_clear)
17421746
print_metric_headers(" ", true);
17431747
if (++num_print_interval == 25)
17441748
num_print_interval = 0;
@@ -2057,6 +2061,8 @@ static const struct option stat_options[] = {
20572061
"(overhead is possible for values <= 100ms)"),
20582062
OPT_INTEGER(0, "interval-count", &stat_config.times,
20592063
"print counts for fixed number of times"),
2064+
OPT_BOOLEAN(0, "interval-clear", &interval_clear,
2065+
"clear screen in between new interval"),
20602066
OPT_UINTEGER(0, "timeout", &stat_config.timeout,
20612067
"stop workload and print counts after a timeout period in ms (>= 10ms)"),
20622068
OPT_SET_UINT(0, "per-socket", &stat_config.aggr_mode,
@@ -2436,14 +2442,13 @@ static int add_default_attributes(void)
24362442
(PERF_COUNT_HW_CACHE_OP_PREFETCH << 8) |
24372443
(PERF_COUNT_HW_CACHE_RESULT_MISS << 16) },
24382444
};
2445+
struct parse_events_error errinfo;
24392446

24402447
/* Set attrs if no event is selected and !null_run: */
24412448
if (null_run)
24422449
return 0;
24432450

24442451
if (transaction_run) {
2445-
struct parse_events_error errinfo;
2446-
24472452
if (pmu_have_event("cpu", "cycles-ct") &&
24482453
pmu_have_event("cpu", "el-start"))
24492454
err = parse_events(evsel_list, transaction_attrs,
@@ -2454,6 +2459,7 @@ static int add_default_attributes(void)
24542459
&errinfo);
24552460
if (err) {
24562461
fprintf(stderr, "Cannot set up transaction events\n");
2462+
parse_events_print_error(&errinfo, transaction_attrs);
24572463
return -1;
24582464
}
24592465
return 0;
@@ -2479,10 +2485,11 @@ static int add_default_attributes(void)
24792485
pmu_have_event("msr", "smi")) {
24802486
if (!force_metric_only)
24812487
metric_only = true;
2482-
err = parse_events(evsel_list, smi_cost_attrs, NULL);
2488+
err = parse_events(evsel_list, smi_cost_attrs, &errinfo);
24832489
} else {
24842490
fprintf(stderr, "To measure SMI cost, it needs "
24852491
"msr/aperf/, msr/smi/ and cpu/cycles/ support\n");
2492+
parse_events_print_error(&errinfo, smi_cost_attrs);
24862493
return -1;
24872494
}
24882495
if (err) {
@@ -2517,12 +2524,13 @@ static int add_default_attributes(void)
25172524
if (topdown_attrs[0] && str) {
25182525
if (warn)
25192526
arch_topdown_group_warn();
2520-
err = parse_events(evsel_list, str, NULL);
2527+
err = parse_events(evsel_list, str, &errinfo);
25212528
if (err) {
25222529
fprintf(stderr,
25232530
"Cannot set up top down events %s: %d\n",
25242531
str, err);
25252532
free(str);
2533+
parse_events_print_error(&errinfo, str);
25262534
return -1;
25272535
}
25282536
} else {

tools/perf/ui/gtk/hists.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
382382
gtk_tree_store_set(store, &iter, col_idx++, s, -1);
383383
}
384384

385-
if (hists__has_callchains(hists) &&
385+
if (hist_entry__has_callchains(h) &&
386386
symbol_conf.use_callchain && hists__has(hists, sym)) {
387387
if (callchain_param.mode == CHAIN_GRAPH_REL)
388388
total = symbol_conf.cumulate_callchain ?

tools/perf/util/hist.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,11 @@ void hists__delete_entries(struct hists *hists)
370370

371371
static int hist_entry__init(struct hist_entry *he,
372372
struct hist_entry *template,
373-
bool sample_self)
373+
bool sample_self,
374+
size_t callchain_size)
374375
{
375376
*he = *template;
377+
he->callchain_size = callchain_size;
376378

377379
if (symbol_conf.cumulate_callchain) {
378380
he->stat_acc = malloc(sizeof(he->stat));
@@ -473,7 +475,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
473475

474476
he = ops->new(callchain_size);
475477
if (he) {
476-
err = hist_entry__init(he, template, sample_self);
478+
err = hist_entry__init(he, template, sample_self, callchain_size);
477479
if (err) {
478480
ops->free(he);
479481
he = NULL;
@@ -619,9 +621,11 @@ __hists__add_entry(struct hists *hists,
619621
.raw_data = sample->raw_data,
620622
.raw_size = sample->raw_size,
621623
.ops = ops,
622-
};
624+
}, *he = hists__findnew_entry(hists, &entry, al, sample_self);
623625

624-
return hists__findnew_entry(hists, &entry, al, sample_self);
626+
if (!hists->has_callchains && he && he->callchain_size != 0)
627+
hists->has_callchains = true;
628+
return he;
625629
}
626630

627631
struct hist_entry *hists__add_entry(struct hists *hists,

tools/perf/util/hist.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ struct hists {
8585
struct events_stats stats;
8686
u64 event_stream;
8787
u16 col_len[HISTC_NR_COLS];
88+
bool has_callchains;
8889
int socket_filter;
8990
struct perf_hpp_list *hpp_list;
9091
struct list_head hpp_formats;
@@ -222,8 +223,7 @@ static inline struct hists *evsel__hists(struct perf_evsel *evsel)
222223

223224
static __pure inline bool hists__has_callchains(struct hists *hists)
224225
{
225-
const struct perf_evsel *evsel = hists_to_evsel(hists);
226-
return evsel__has_callchain(evsel);
226+
return hists->has_callchains;
227227
}
228228

229229
int hists__init(void);

tools/perf/util/parse-events.y

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,16 @@ event_def: event_pmu |
227227
event_pmu:
228228
PE_NAME opt_pmu_config
229229
{
230+
struct parse_events_state *parse_state = _parse_state;
231+
struct parse_events_error *error = parse_state->error;
230232
struct list_head *list, *orig_terms, *terms;
231233

232234
if (parse_events_copy_term_list($2, &orig_terms))
233235
YYABORT;
234236

237+
if (error)
238+
error->idx = @1.first_column;
239+
235240
ALLOC_LIST(list);
236241
if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
237242
struct perf_pmu *pmu = NULL;

tools/perf/util/sort.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ struct hist_entry {
112112

113113
char level;
114114
u8 filtered;
115+
116+
u16 callchain_size;
115117
union {
116118
/*
117119
* Since perf diff only supports the stdio output, TUI
@@ -153,7 +155,7 @@ struct hist_entry {
153155

154156
static __pure inline bool hist_entry__has_callchains(struct hist_entry *he)
155157
{
156-
return hists__has_callchains(he->hists);
158+
return he->callchain_size != 0;
157159
}
158160

159161
static inline bool hist_entry__has_pairs(struct hist_entry *he)

0 commit comments

Comments
 (0)