diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt index 4da06215163a..fe0abef2bd13 100644 --- a/tools/perf/Documentation/perf-sched.txt +++ b/tools/perf/Documentation/perf-sched.txt @@ -95,8 +95,10 @@ There are several variants of 'perf sched': events, ``try_to_wakeup()`` call among others. This is useful in understanding the scheduler behavior for the workload. - Note: The tool will not give correct results if there is topological reordering or - online/offline of cpus in between capturing snapshots of `/proc/schedstat`. + Note: Reports reject incomplete snapshots and mismatched CPU or domain IDs. + Topology changes that retain the same IDs, including CPUs going offline + and returning online between snapshots, cannot be detected and may still + produce incorrect results. Example usage: perf sched stats record -- sleep 1 diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 6e8bf619c86c..39571f03eda2 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -4281,18 +4281,18 @@ struct schedstat_cpu { }; static struct list_head cpu_head = LIST_HEAD_INIT(cpu_head); -static struct schedstat_cpu *cpu_second_pass; -static struct schedstat_domain *domain_second_pass; +static struct list_head *cpu_second_pass; +static struct list_head *domain_second_pass; +static u64 schedstat_timestamp; static bool after_workload_flag; static bool verbose_field; static void free_schedstat(struct list_head *head); -static void store_schedstat_cpu_diff(struct schedstat_cpu *after_workload) +static void store_schedstat_cpu_diff(struct perf_record_schedstat_cpu *before, + struct perf_record_schedstat_cpu *after) { - struct perf_record_schedstat_cpu *before = cpu_second_pass->cpu_data; - struct perf_record_schedstat_cpu *after = after_workload->cpu_data; - __u16 version = after_workload->cpu_data->version; + __u16 version = after->version; #define CPU_FIELD(_type, _name, _desc, _format, _is_pct, _pct_of, _ver) \ (before->_ver._name = after->_ver._name - before->_ver._name) @@ -4308,11 +4308,10 @@ static void store_schedstat_cpu_diff(struct schedstat_cpu *after_workload) #undef CPU_FIELD } -static void store_schedstat_domain_diff(struct schedstat_domain *after_workload) +static void store_schedstat_domain_diff(struct perf_record_schedstat_domain *before, + struct perf_record_schedstat_domain *after) { - struct perf_record_schedstat_domain *before = domain_second_pass->domain_data; - struct perf_record_schedstat_domain *after = after_workload->domain_data; - __u16 version = after_workload->domain_data->version; + __u16 version = after->version; #define DOMAIN_FIELD(_type, _name, _desc, _format, _is_jiffies, _ver) \ (before->_ver._name = after->_ver._name - before->_ver._name) @@ -4814,12 +4813,35 @@ static int show_schedstat_data(struct list_head *head1, struct cpu_domain_map ** * other after completion of the workload. The above linked list stores the diff of the cpu and * domain statistics. */ +static int schedstat_snapshot_error(void) +{ + pr_err("Incompatible or incomplete schedstat snapshots\n"); + return -EINVAL; +} + +static bool schedstat_domains_complete(void) +{ + struct schedstat_cpu *cpu; + + if (!domain_second_pass) + return true; + cpu = list_entry(cpu_second_pass, struct schedstat_cpu, cpu_list); + return domain_second_pass == &cpu->domain_head; +} + +static int schedstat_snapshots_complete(void) +{ + if (!after_workload_flag || !cpu_second_pass || + cpu_second_pass->next != &cpu_head || !schedstat_domains_complete()) + return schedstat_snapshot_error(); + return 0; +} + static int perf_sched__process_schedstat(const struct perf_tool *tool __maybe_unused, struct perf_session *session __maybe_unused, union perf_event *event) { struct perf_cpu this_cpu; - static __u32 initial_cpu; switch (event->header.type) { case PERF_RECORD_SCHEDSTAT_CPU: @@ -4836,63 +4858,91 @@ static int perf_sched__process_schedstat(const struct perf_tool *tool __maybe_un return 0; if (event->header.type == PERF_RECORD_SCHEDSTAT_CPU) { - struct schedstat_cpu *temp = zalloc(sizeof(*temp)); + struct perf_record_schedstat_cpu *data = &event->schedstat_cpu; + struct schedstat_cpu *cpu; - if (!temp) - return -ENOMEM; - - temp->cpu_data = zalloc(sizeof(*temp->cpu_data)); - if (!temp->cpu_data) - return -ENOMEM; - - memcpy(temp->cpu_data, &event->schedstat_cpu, sizeof(*temp->cpu_data)); - - if (!list_empty(&cpu_head) && temp->cpu_data->cpu == initial_cpu) - after_workload_flag = true; - - if (!after_workload_flag) { - if (list_empty(&cpu_head)) - initial_cpu = temp->cpu_data->cpu; - - list_add_tail(&temp->cpu_list, &cpu_head); - INIT_LIST_HEAD(&temp->domain_head); - } else { - if (temp->cpu_data->cpu == initial_cpu) { - cpu_second_pass = list_first_entry(&cpu_head, struct schedstat_cpu, - cpu_list); - cpu_second_pass->cpu_data->timestamp = - temp->cpu_data->timestamp - cpu_second_pass->cpu_data->timestamp; - } else { - cpu_second_pass = list_next_entry(cpu_second_pass, cpu_list); + if (list_empty(&cpu_head)) { + after_workload_flag = false; + cpu_second_pass = &cpu_head; + domain_second_pass = NULL; + schedstat_timestamp = data->timestamp; + } else if (!after_workload_flag) { + cpu = list_last_entry(&cpu_head, struct schedstat_cpu, cpu_list); + /* Snapshots share a timestamp and list CPUs in increasing order. */ + if (data->timestamp != schedstat_timestamp || + data->cpu <= cpu->cpu_data->cpu) { + after_workload_flag = true; + schedstat_timestamp = data->timestamp; } - domain_second_pass = list_first_entry(&cpu_second_pass->domain_head, - struct schedstat_domain, domain_list); - store_schedstat_cpu_diff(temp); - free(temp->cpu_data); - free(temp); } - } else if (event->header.type == PERF_RECORD_SCHEDSTAT_DOMAIN) { - struct schedstat_cpu *cpu_tail; - struct schedstat_domain *temp = zalloc(sizeof(*temp)); - if (!temp) - return -ENOMEM; - - temp->domain_data = zalloc(sizeof(*temp->domain_data)); - if (!temp->domain_data) - return -ENOMEM; - - memcpy(temp->domain_data, &event->schedstat_domain, sizeof(*temp->domain_data)); - - if (!after_workload_flag) { - cpu_tail = list_last_entry(&cpu_head, struct schedstat_cpu, cpu_list); - list_add_tail(&temp->domain_list, &cpu_tail->domain_head); - } else { - store_schedstat_domain_diff(temp); - domain_second_pass = list_next_entry(domain_second_pass, domain_list); - free(temp->domain_data); - free(temp); + if (after_workload_flag) { + if (data->timestamp != schedstat_timestamp || !schedstat_domains_complete()) + return schedstat_snapshot_error(); + cpu_second_pass = cpu_second_pass->next; + if (cpu_second_pass == &cpu_head) + return schedstat_snapshot_error(); + cpu = list_entry(cpu_second_pass, struct schedstat_cpu, cpu_list); + if (data->cpu != cpu->cpu_data->cpu || + data->version != cpu->cpu_data->version || + data->timestamp < cpu->cpu_data->timestamp) + return schedstat_snapshot_error(); + cpu->cpu_data->timestamp = data->timestamp - cpu->cpu_data->timestamp; + store_schedstat_cpu_diff(cpu->cpu_data, data); + domain_second_pass = cpu->domain_head.next; + return 0; } + + cpu = zalloc(sizeof(*cpu)); + if (!cpu) + return -ENOMEM; + cpu->cpu_data = memdup(data, sizeof(*data)); + if (!cpu->cpu_data) { + free(cpu); + return -ENOMEM; + } + INIT_LIST_HEAD(&cpu->domain_head); + list_add_tail(&cpu->cpu_list, &cpu_head); + } else { + struct perf_record_schedstat_domain *data = &event->schedstat_domain; + struct schedstat_domain *domain; + struct schedstat_cpu *cpu; + + if (list_empty(&cpu_head) || data->timestamp != schedstat_timestamp) + return schedstat_snapshot_error(); + if (after_workload_flag) { + cpu = list_entry(cpu_second_pass, struct schedstat_cpu, cpu_list); + if (domain_second_pass == &cpu->domain_head) + return schedstat_snapshot_error(); + domain = list_entry(domain_second_pass, struct schedstat_domain, + domain_list); + if (data->cpu != domain->domain_data->cpu || + data->domain != domain->domain_data->domain || + data->version != domain->domain_data->version) + return schedstat_snapshot_error(); + store_schedstat_domain_diff(domain->domain_data, data); + domain_second_pass = domain_second_pass->next; + return 0; + } + + cpu = list_last_entry(&cpu_head, struct schedstat_cpu, cpu_list); + if (data->cpu != cpu->cpu_data->cpu || data->version != cpu->cpu_data->version) + return schedstat_snapshot_error(); + if (!list_empty(&cpu->domain_head)) { + domain = list_last_entry(&cpu->domain_head, struct schedstat_domain, + domain_list); + if (data->domain <= domain->domain_data->domain) + return schedstat_snapshot_error(); + } + domain = zalloc(sizeof(*domain)); + if (!domain) + return -ENOMEM; + domain->domain_data = memdup(data, sizeof(*data)); + if (!domain->domain_data) { + free(domain); + return -ENOMEM; + } + list_add_tail(&domain->domain_list, &cpu->domain_head); } return 0; @@ -4947,6 +4997,8 @@ static int perf_sched__schedstat_report(struct perf_sched *sched) user_requested_cpus = evlist__core(session->evlist)->user_requested_cpus; err = perf_session__process_events(session); + if (!err) + err = schedstat_snapshots_complete(); if (!err) { setup_pager(); @@ -4976,7 +5028,7 @@ static int perf_sched__schedstat_diff(struct perf_sched *sched, struct list_head cpu_head_ses0, cpu_head_ses1; struct perf_session *session[2]; struct perf_data data[2] = {0}; - int ret = 0, err = 0; + int ret = 0; static const char *defaults[] = { "perf.data.old", "perf.data", @@ -5009,8 +5061,10 @@ static int perf_sched__schedstat_diff(struct perf_sched *sched, goto out_delete_ses0; } - err = perf_session__process_events(session[0]); - if (err) { + ret = perf_session__process_events(session[0]); + if (!ret) + ret = schedstat_snapshots_complete(); + if (ret) { free_schedstat(&cpu_head); goto out_delete_ses0; } @@ -5028,8 +5082,10 @@ static int perf_sched__schedstat_diff(struct perf_sched *sched, goto out_delete_ses1; } - err = perf_session__process_events(session[1]); - if (err) { + ret = perf_session__process_events(session[1]); + if (!ret) + ret = schedstat_snapshots_complete(); + if (ret) { free_schedstat(&cpu_head); goto out_delete_ses1; } @@ -5152,6 +5208,9 @@ static int perf_sched__schedstat_live(struct perf_sched *sched, user_requested_cpus); if (err) goto out; + err = schedstat_snapshots_complete(); + if (err) + goto out; setup_pager(); diff --git a/tools/perf/tests/shell/schedstat_snapshots.sh b/tools/perf/tests/shell/schedstat_snapshots.sh new file mode 100755 index 000000000000..a4bdf2673d43 --- /dev/null +++ b/tools/perf/tests/shell/schedstat_snapshots.sh @@ -0,0 +1,155 @@ +#!/bin/sh +# Validate CPU and domain pairing in perf sched stats snapshots +# SPDX-License-Identifier: GPL-2.0 + +set -e + +# shellcheck source=lib/setup_python.sh +. "$(dirname "$0")/lib/setup_python.sh" + +if ! perf version --build-options | grep -q 'libtraceevent:.*on'; then + echo "[Skip] perf sched requires libtraceevent" + exit 2 +fi + +$PYTHON - <<'PY' +import os +import re +import struct +import subprocess +import sys +import tempfile + +# Native-endian perf.data with only NRCPUS and CPU_DOMAIN_INFO features. +endian = '<' if sys.byteorder == 'little' else '>' + + +def pack(fmt, *values): + return struct.pack(endian + fmt, *values) + + +def string(value): + data = value.encode() + b'\0' + return pack('I', len(data)) + data + + +def cpu(cpu_id, timestamp, value, version): + return pack('IHHQIHH6I3Q', 85, 0, 72, timestamp, cpu_id, version, 0, + *([value] * 9)) + + +def domain(cpu_id, domain_id, timestamp, value, version): + # All supported versions use the largest union member's record size. + return pack('IHHQIHH45I4x', 86, 0, 208, timestamp, cpu_id, version, + domain_id, *([value] * 45)) + + +def snapshot(timestamp, value, version=17, cpus=(0, 1, 2), domains=(0, 1)): + records = [] + for cpu_id in cpus: + records.append(cpu(cpu_id, timestamp, value + cpu_id * 100, version)) + for domain_id in domains: + records.append(domain(cpu_id, domain_id, timestamp, + value + cpu_id * 100, version)) + return records + + +def write_file(path, records, version=17): + metadata = pack('II', version, 2) + for cpu_id in range(3): + metadata += pack('II', cpu_id, 2) + for domain_id in range(2): + metadata += pack('I', domain_id) + if version >= 17: + metadata += string('SMT' if domain_id == 0 else 'MC') + metadata += string('7') + string('0-2') + features = [pack('II', 3, 3), metadata] + data = b''.join(records) + offset = 104 + len(data) + 16 * len(features) + sections = b'' + for feature in features: + sections += pack('QQ', offset, len(feature)) + offset += len(feature) + header = pack('13Q', 0x32454c4946524550, 104, 144, 104, 0, + 104, len(data), 0, 0, (1 << 7) | (1 << 32), 0, 0, 0) + with open(path, 'wb') as output: + output.write(header + data + sections + b''.join(features)) + + +def run(args, valid, domains=True): + result = subprocess.run(['perf', 'sched', 'stats'] + args, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + text=True, timeout=10) + if valid: + assert result.returncode == 0, result.stderr + assert re.search(r'^yld_count\s+:\s+10\b', result.stdout, re.M), result.stdout + if domains: + assert re.search(r'^busy_lb_count\s+:\s+10\b', result.stdout, re.M), result.stdout + else: + assert 'busy_lb_count' not in result.stdout, result.stdout + else: + assert result.returncode > 0, (args, result.returncode, result.stdout) + assert 'Incompatible or incomplete schedstat snapshots' in result.stderr + assert not result.stdout, result.stdout + assert 'Sanitizer' not in result.stderr, result.stderr + + +with tempfile.TemporaryDirectory(prefix='perf-schedstat-') as directory: + good = os.path.join(directory, 'good.data') + test = os.path.join(directory, 'test.data') + before = snapshot(100, 1000) + after = snapshot(200, 1010) + write_file(good, before + after) + + for version in (15, 16, 17): + for timestamp in (100, 200): + write_file(test, snapshot(100, 1000, version) + + snapshot(timestamp, 1010, version), version) + run(['report', '-C', '0,1,2', '-i', test], True) + run(['diff', test, test], True) + print('Matching snapshots, including equal timestamps: [Success]') + + write_file(test, snapshot(100, 1000, domains=()) + + snapshot(200, 1010, domains=())) + run(['report', '-C', '0,1,2', '-i', test], True, domains=False) + run(['diff', test, test], True, domains=False) + print('CPUs without domains: [Success]') + + write_file(test, before + snapshot(200, 1010, cpus=(0, 1))) + run(['report', '-C', '0,1', '-i', test], True) + run(['report', '-C', '1', '-i', good], True) + write_file(test, snapshot(100, 1000, cpus=(0, 1)) + + snapshot(200, 1010, cpus=(0, 1))) + run(['diff', good, test], True) + print('CPU filtering and different CPU sets across files: [Success]') + + write_file(test, snapshot(100, 0xfffffffa, cpus=(0,)) + + snapshot(200, 4, cpus=(0,))) + run(['report', '-C', '0', '-i', test], True) + print('Wrapping 32-bit counters: [Success]') + + cases = { + 'missing first CPU': before + snapshot(200, 1010, cpus=(1, 2)), + 'missing middle CPU': before + snapshot(200, 1010, cpus=(0, 2)), + 'missing last CPU': before + snapshot(200, 1010, cpus=(0, 1)), + 'added CPU': snapshot(100, 1000, cpus=(0, 1)) + after, + 'reordered CPUs': before + snapshot(200, 1010, cpus=(0, 2, 1)), + 'equal timestamp, missing first CPU': before + snapshot(100, 1010, cpus=(1, 2)), + 'missing first domain': before + snapshot(200, 1010, domains=(1,)), + 'missing last domain': before + after[:-1], + 'added domain': snapshot(100, 1000, domains=(0,)) + after, + 'reordered domains': before + snapshot(200, 1010, domains=(1, 0)), + 'domain without CPU': before[1:] + after, + 'wrong domain CPU': before + [after[0], domain(1, 0, 200, 1010, 17)] + after[2:], + 'changed version': before + snapshot(200, 1010, version=16), + 'backwards timestamp': before + snapshot(50, 1010), + 'third snapshot': before + after + snapshot(300, 1020), + 'missing second snapshot': before, + } + for name, records in cases.items(): + write_file(test, records) + run(['report', '-C', '0,1,2', '-i', test], False) + run(['diff', test, good], False) + run(['diff', good, test], False) + print(name + ': [Success]') +PY