From 999eea92e8d7a1ffa83f7dc89c83d8ed1e746fa9 Mon Sep 17 00:00:00 2001 From: Thomas Richter Date: Mon, 8 Jan 2024 09:40:09 +0100 Subject: perf test: raise limit to 20 percent for perf_stat_--bpf-counters_test This test case often fails on s390 (about 2 out of 10) because the 10% percent limit on the difference between --bpf-counters event counting and s390 hardware counting is more than 10% in all failure cases. Raise the limit to 20% on s390 and the test case succeeds. Signed-off-by: Thomas Richter Acked-by: Namhyung Kim Cc: gor@linux.ibm.com Cc: hca@linux.ibm.com Cc: sumanthk@linux.ibm.com Cc: svens@linux.ibm.com Link: https://lore.kernel.org/r/20240108084009.3959211-1-tmricht@linux.ibm.com Signed-off-by: Namhyung Kim --- tools/perf/tests/shell/stat_bpf_counters.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh index a87bb2814b4c..2d9209874774 100755 --- a/tools/perf/tests/shell/stat_bpf_counters.sh +++ b/tools/perf/tests/shell/stat_bpf_counters.sh @@ -4,19 +4,19 @@ set -e -# check whether $2 is within +/- 10% of $1 +# check whether $2 is within +/- 20% of $1 compare_number() { first_num=$1 second_num=$2 - # upper bound is first_num * 110% - upper=$(expr $first_num + $first_num / 10 ) - # lower bound is first_num * 90% - lower=$(expr $first_num - $first_num / 10 ) + # upper bound is first_num * 120% + upper=$(expr $first_num + $first_num / 5 ) + # lower bound is first_num * 80% + lower=$(expr $first_num - $first_num / 5 ) if [ $second_num -gt $upper ] || [ $second_num -lt $lower ]; then - echo "The difference between $first_num and $second_num are greater than 10%." + echo "The difference between $first_num and $second_num are greater than 20%." exit 1 fi } -- cgit v1.2.3 From ac668d529fca730f318db3bfa7141743bf75976d Mon Sep 17 00:00:00 2001 From: James Clark Date: Tue, 23 Jan 2024 16:39:02 +0000 Subject: perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in Even though this is a frame pointer unwind test, it's testing that a frame pointer stack can be augmented correctly with a partial Dwarf unwind. So add a feature check so that this test skips instead of fails if Dwarf unwinding isn't present. Signed-off-by: James Clark Reviewed-by: Ian Rogers Cc: Mark Rutland Cc: Spoorthy S Cc: Kajol Jain Link: https://lore.kernel.org/r/20240123163903.350306-3-james.clark@arm.com Signed-off-by: Namhyung Kim --- tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh index e342e6c8aa50..83b53591b1ea 100755 --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh @@ -8,6 +8,12 @@ shelldir=$(dirname "$0") lscpu | grep -q "aarch64" || exit 2 +if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF +then + echo "Skipping, no dwarf unwind support" + exit 2 +fi + skip_test_missing_symbol leafloop PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX) -- cgit v1.2.3 From 8f95b29c73e181274759c3f9483ba7f875447e3c Mon Sep 17 00:00:00 2001 From: Weilin Wang Date: Tue, 30 Jan 2024 10:09:07 -0800 Subject: perf test: Simplify metric value validation test final report The original test report was too complicated to read with information that not really useful. This new update simplify the report which should largely improve the readibility. Signed-off-by: Weilin Wang Reviewed-by: Ian Rogers Cc: Caleb Biggers Cc: Perry Taylor Cc: Samantha Alt Cc: Kan Liang Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240130180907.639729-1-weilin.wang@intel.com --- .../perf/tests/shell/lib/perf_metric_validation.py | 231 +++++++++++---------- tools/perf/tests/shell/stat_metrics_values.sh | 4 +- 2 files changed, 127 insertions(+), 108 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/lib/perf_metric_validation.py b/tools/perf/tests/shell/lib/perf_metric_validation.py index 50a34a9cc040..a2d235252183 100644 --- a/tools/perf/tests/shell/lib/perf_metric_validation.py +++ b/tools/perf/tests/shell/lib/perf_metric_validation.py @@ -1,4 +1,4 @@ -#SPDX-License-Identifier: GPL-2.0 +# SPDX-License-Identifier: GPL-2.0 import re import csv import json @@ -6,36 +6,61 @@ import argparse from pathlib import Path import subprocess + +class TestError: + def __init__(self, metric: list[str], wl: str, value: list[float], low: float, up=float('nan'), description=str()): + self.metric: list = metric # multiple metrics in relationship type tests + self.workloads = [wl] # multiple workloads possible + self.collectedValue: list = value + self.valueLowBound = low + self.valueUpBound = up + self.description = description + + def __repr__(self) -> str: + if len(self.metric) > 1: + return "\nMetric Relationship Error: \tThe collected value of metric {0}\n\ + \tis {1} in workload(s): {2} \n\ + \tbut expected value range is [{3}, {4}]\n\ + \tRelationship rule description: \'{5}\'".format(self.metric, self.collectedValue, self.workloads, + self.valueLowBound, self.valueUpBound, self.description) + elif len(self.collectedValue) == 0: + return "\nNo Metric Value Error: \tMetric {0} returns with no value \n\ + \tworkload(s): {1}".format(self.metric, self.workloads) + else: + return "\nWrong Metric Value Error: \tThe collected value of metric {0}\n\ + \tis {1} in workload(s): {2}\n\ + \tbut expected value range is [{3}, {4}]"\ + .format(self.metric, self.collectedValue, self.workloads, + self.valueLowBound, self.valueUpBound) + + class Validator: def __init__(self, rulefname, reportfname='', t=5, debug=False, datafname='', fullrulefname='', workload='true', metrics=''): self.rulefname = rulefname self.reportfname = reportfname self.rules = None - self.collectlist:str = metrics + self.collectlist: str = metrics self.metrics = self.__set_metrics(metrics) self.skiplist = set() self.tolerance = t self.workloads = [x for x in workload.split(",") if x] - self.wlidx = 0 # idx of current workloads - self.allresults = dict() # metric results of all workload - self.allignoremetrics = dict() # metrics with no results or negative results - self.allfailtests = dict() + self.wlidx = 0 # idx of current workloads + self.allresults = dict() # metric results of all workload self.alltotalcnt = dict() self.allpassedcnt = dict() - self.allerrlist = dict() - self.results = dict() # metric results of current workload + self.results = dict() # metric results of current workload # vars for test pass/failure statistics - self.ignoremetrics= set() # metrics with no results or negative results, neg result counts as a failed test - self.failtests = dict() + # metrics with no results or negative results, neg result counts failed tests + self.ignoremetrics = set() self.totalcnt = 0 self.passedcnt = 0 # vars for errors self.errlist = list() # vars for Rule Generator - self.pctgmetrics = set() # Percentage rule + self.pctgmetrics = set() # Percentage rule # vars for debug self.datafname = datafname @@ -69,10 +94,10 @@ class Validator: ensure_ascii=True, indent=4) - def get_results(self, idx:int = 0): + def get_results(self, idx: int = 0): return self.results[idx] - def get_bounds(self, lb, ub, error, alias={}, ridx:int = 0) -> list: + def get_bounds(self, lb, ub, error, alias={}, ridx: int = 0) -> list: """ Get bounds and tolerance from lb, ub, and error. If missing lb, use 0.0; missing ub, use float('inf); missing error, use self.tolerance. @@ -85,7 +110,7 @@ class Validator: tolerance, denormalized base on upper bound value """ # init ubv and lbv to invalid values - def get_bound_value (bound, initval, ridx): + def get_bound_value(bound, initval, ridx): val = initval if isinstance(bound, int) or isinstance(bound, float): val = bound @@ -113,10 +138,10 @@ class Validator: return lbv, ubv, denormerr - def get_value(self, name:str, ridx:int = 0) -> list: + def get_value(self, name: str, ridx: int = 0) -> list: """ Get value of the metric from self.results. - If result of this metric is not provided, the metric name will be added into self.ignoremetics and self.errlist. + If result of this metric is not provided, the metric name will be added into self.ignoremetics. All future test(s) on this metric will fail. @param name: name of the metric @@ -142,7 +167,7 @@ class Validator: Check if metrics value are non-negative. One metric is counted as one test. Failure: when metric value is negative or not provided. - Metrics with negative value will be added into the self.failtests['PositiveValueTest'] and self.ignoremetrics. + Metrics with negative value will be added into self.ignoremetrics. """ negmetric = dict() pcnt = 0 @@ -155,25 +180,27 @@ class Validator: else: pcnt += 1 tcnt += 1 + # The first round collect_perf() run these metrics with simple workload + # "true". We give metrics a second chance with a longer workload if less + # than 20 metrics failed positive test. if len(rerun) > 0 and len(rerun) < 20: second_results = dict() self.second_test(rerun, second_results) for name, val in second_results.items(): - if name not in negmetric: continue + if name not in negmetric: + continue if val >= 0: del negmetric[name] pcnt += 1 - self.failtests['PositiveValueTest']['Total Tests'] = tcnt - self.failtests['PositiveValueTest']['Passed Tests'] = pcnt if len(negmetric.keys()): self.ignoremetrics.update(negmetric.keys()) - negmessage = ["{0}(={1:.4f})".format(name, val) for name, val in negmetric.items()] - self.failtests['PositiveValueTest']['Failed Tests'].append({'NegativeValue': negmessage}) + self.errlist.extend( + [TestError([m], self.workloads[self.wlidx], negmetric[m], 0) for m in negmetric.keys()]) return - def evaluate_formula(self, formula:str, alias:dict, ridx:int = 0): + def evaluate_formula(self, formula: str, alias: dict, ridx: int = 0): """ Evaluate the value of formula. @@ -187,10 +214,11 @@ class Validator: sign = "+" f = str() - #TODO: support parenthesis? + # TODO: support parenthesis? for i in range(len(formula)): if i+1 == len(formula) or formula[i] in ('+', '-', '*', '/'): - s = alias[formula[b:i]] if i+1 < len(formula) else alias[formula[b:]] + s = alias[formula[b:i]] if i + \ + 1 < len(formula) else alias[formula[b:]] v = self.get_value(s, ridx) if not v: errs.append(s) @@ -228,49 +256,49 @@ class Validator: alias = dict() for m in rule['Metrics']: alias[m['Alias']] = m['Name'] - lbv, ubv, t = self.get_bounds(rule['RangeLower'], rule['RangeUpper'], rule['ErrorThreshold'], alias, ridx=rule['RuleIndex']) - val, f = self.evaluate_formula(rule['Formula'], alias, ridx=rule['RuleIndex']) + lbv, ubv, t = self.get_bounds( + rule['RangeLower'], rule['RangeUpper'], rule['ErrorThreshold'], alias, ridx=rule['RuleIndex']) + val, f = self.evaluate_formula( + rule['Formula'], alias, ridx=rule['RuleIndex']) + + lb = rule['RangeLower'] + ub = rule['RangeUpper'] + if isinstance(lb, str): + if lb in alias: + lb = alias[lb] + if isinstance(ub, str): + if ub in alias: + ub = alias[ub] + if val == -1: - self.failtests['RelationshipTest']['Failed Tests'].append({'RuleIndex': rule['RuleIndex'], 'Description':f}) + self.errlist.append(TestError([m['Name'] for m in rule['Metrics']], self.workloads[self.wlidx], [], + lb, ub, rule['Description'])) elif not self.check_bound(val, lbv, ubv, t): - lb = rule['RangeLower'] - ub = rule['RangeUpper'] - if isinstance(lb, str): - if lb in alias: - lb = alias[lb] - if isinstance(ub, str): - if ub in alias: - ub = alias[ub] - self.failtests['RelationshipTest']['Failed Tests'].append({'RuleIndex': rule['RuleIndex'], 'Formula':f, - 'RangeLower': lb, 'LowerBoundValue': self.get_value(lb), - 'RangeUpper': ub, 'UpperBoundValue':self.get_value(ub), - 'ErrorThreshold': t, 'CollectedValue': val}) + self.errlist.append(TestError([m['Name'] for m in rule['Metrics']], self.workloads[self.wlidx], [val], + lb, ub, rule['Description'])) else: self.passedcnt += 1 - self.failtests['RelationshipTest']['Passed Tests'] += 1 self.totalcnt += 1 - self.failtests['RelationshipTest']['Total Tests'] += 1 return - # Single Metric Test - def single_test(self, rule:dict): + def single_test(self, rule: dict): """ Validate if the metrics are in the required value range. eg. lower_bound <= metrics_value <= upper_bound One metric is counted as one test in this type of test. One rule may include one or more metrics. Failure: when the metric value not provided or the value is outside the bounds. - This test updates self.total_cnt and records failed tests in self.failtest['SingleMetricTest']. + This test updates self.total_cnt. @param rule: dict with metrics to validate and the value range requirement """ - lbv, ubv, t = self.get_bounds(rule['RangeLower'], rule['RangeUpper'], rule['ErrorThreshold']) + lbv, ubv, t = self.get_bounds( + rule['RangeLower'], rule['RangeUpper'], rule['ErrorThreshold']) metrics = rule['Metrics'] passcnt = 0 totalcnt = 0 - faillist = list() failures = dict() rerun = list() for m in metrics: @@ -286,25 +314,20 @@ class Validator: second_results = dict() self.second_test(rerun, second_results) for name, val in second_results.items(): - if name not in failures: continue + if name not in failures: + continue if self.check_bound(val, lbv, ubv, t): passcnt += 1 del failures[name] else: - failures[name] = val + failures[name] = [val] self.results[0][name] = val self.totalcnt += totalcnt self.passedcnt += passcnt - self.failtests['SingleMetricTest']['Total Tests'] += totalcnt - self.failtests['SingleMetricTest']['Passed Tests'] += passcnt if len(failures.keys()) != 0: - faillist = [{'MetricName':name, 'CollectedValue':val} for name, val in failures.items()] - self.failtests['SingleMetricTest']['Failed Tests'].append({'RuleIndex':rule['RuleIndex'], - 'RangeLower': rule['RangeLower'], - 'RangeUpper': rule['RangeUpper'], - 'ErrorThreshold':rule['ErrorThreshold'], - 'Failure':faillist}) + self.errlist.extend([TestError([name], self.workloads[self.wlidx], val, + rule['RangeLower'], rule['RangeUpper']) for name, val in failures.items()]) return @@ -312,19 +335,11 @@ class Validator: """ Create final report and write into a JSON file. """ - alldata = list() - for i in range(0, len(self.workloads)): - reportstas = {"Total Rule Count": self.alltotalcnt[i], "Passed Rule Count": self.allpassedcnt[i]} - data = {"Metric Validation Statistics": reportstas, "Tests in Category": self.allfailtests[i], - "Errors":self.allerrlist[i]} - alldata.append({"Workload": self.workloads[i], "Report": data}) - - json_str = json.dumps(alldata, indent=4) - print("Test validation finished. Final report: ") - print(json_str) + print(self.errlist) if self.debug: - allres = [{"Workload": self.workloads[i], "Results": self.allresults[i]} for i in range(0, len(self.workloads))] + allres = [{"Workload": self.workloads[i], "Results": self.allresults[i]} + for i in range(0, len(self.workloads))] self.json_dump(allres, self.datafname) def check_rule(self, testtype, metric_list): @@ -342,13 +357,13 @@ class Validator: return True # Start of Collector and Converter - def convert(self, data: list, metricvalues:dict): + def convert(self, data: list, metricvalues: dict): """ Convert collected metric data from the -j output to dict of {metric_name:value}. """ for json_string in data: try: - result =json.loads(json_string) + result = json.loads(json_string) if "metric-unit" in result and result["metric-unit"] != "(null)" and result["metric-unit"] != "": name = result["metric-unit"].split(" ")[1] if len(result["metric-unit"].split(" ")) > 1 \ else result["metric-unit"] @@ -365,9 +380,10 @@ class Validator: print(" ".join(command)) cmd = subprocess.run(command, stderr=subprocess.PIPE, encoding='utf-8') data = [x+'}' for x in cmd.stderr.split('}\n') if x] + if data[0][0] != '{': + data[0] = data[0][data[0].find('{'):] return data - def collect_perf(self, workload: str): """ Collect metric data with "perf stat -M" on given workload with -a and -j. @@ -385,14 +401,18 @@ class Validator: if rule["TestType"] == "RelationshipTest": metrics = [m["Name"] for m in rule["Metrics"]] if not any(m not in collectlist[0] for m in metrics): - collectlist[rule["RuleIndex"]] = [",".join(list(set(metrics)))] + collectlist[rule["RuleIndex"]] = [ + ",".join(list(set(metrics)))] for idx, metrics in collectlist.items(): - if idx == 0: wl = "true" - else: wl = workload + if idx == 0: + wl = "true" + else: + wl = workload for metric in metrics: data = self._run_perf(metric, wl) - if idx not in self.results: self.results[idx] = dict() + if idx not in self.results: + self.results[idx] = dict() self.convert(data, self.results[idx]) return @@ -412,7 +432,8 @@ class Validator: 2) create metric name list """ command = ['perf', 'list', '-j', '--details', 'metrics'] - cmd = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8') + cmd = subprocess.run(command, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, encoding='utf-8') try: data = json.loads(cmd.stdout) for m in data: @@ -453,12 +474,12 @@ class Validator: rules = data['RelationshipRules'] self.skiplist = set([name.lower() for name in data['SkipList']]) self.rules = self.remove_unsupported_rules(rules) - pctgrule = {'RuleIndex':0, - 'TestType':'SingleMetricTest', - 'RangeLower':'0', + pctgrule = {'RuleIndex': 0, + 'TestType': 'SingleMetricTest', + 'RangeLower': '0', 'RangeUpper': '100', 'ErrorThreshold': self.tolerance, - 'Description':'Metrics in percent unit have value with in [0, 100]', + 'Description': 'Metrics in percent unit have value with in [0, 100]', 'Metrics': [{'Name': m.lower()} for m in self.pctgmetrics]} self.rules.append(pctgrule) @@ -469,8 +490,9 @@ class Validator: idx += 1 if self.debug: - #TODO: need to test and generate file name correctly - data = {'RelationshipRules':self.rules, 'SupportedMetrics': [{"MetricName": name} for name in self.metrics]} + # TODO: need to test and generate file name correctly + data = {'RelationshipRules': self.rules, 'SupportedMetrics': [ + {"MetricName": name} for name in self.metrics]} self.json_dump(data, self.fullrulefname) return @@ -482,20 +504,17 @@ class Validator: @param key: key to the dictionaries (index of self.workloads). ''' self.allresults[key] = self.results - self.allignoremetrics[key] = self.ignoremetrics - self.allfailtests[key] = self.failtests self.alltotalcnt[key] = self.totalcnt self.allpassedcnt[key] = self.passedcnt - self.allerrlist[key] = self.errlist - #Initialize data structures before data validation of each workload + # Initialize data structures before data validation of each workload def _init_data(self): - testtypes = ['PositiveValueTest', 'RelationshipTest', 'SingleMetricTest'] + testtypes = ['PositiveValueTest', + 'RelationshipTest', 'SingleMetricTest'] self.results = dict() - self.ignoremetrics= set() + self.ignoremetrics = set() self.errlist = list() - self.failtests = {k:{'Total Tests':0, 'Passed Tests':0, 'Failed Tests':[]} for k in testtypes} self.totalcnt = 0 self.passedcnt = 0 @@ -525,32 +544,33 @@ class Validator: testtype = r['TestType'] if not self.check_rule(testtype, r['Metrics']): continue - if testtype == 'RelationshipTest': + if testtype == 'RelationshipTest': self.relationship_test(r) elif testtype == 'SingleMetricTest': self.single_test(r) else: print("Unsupported Test Type: ", testtype) - self.errlist.append("Unsupported Test Type from rule: " + r['RuleIndex']) - self._storewldata(i) print("Workload: ", self.workloads[i]) - print("Total metrics collected: ", self.failtests['PositiveValueTest']['Total Tests']) - print("Non-negative metric count: ", self.failtests['PositiveValueTest']['Passed Tests']) print("Total Test Count: ", self.totalcnt) print("Passed Test Count: ", self.passedcnt) - + self._storewldata(i) self.create_report() - return sum(self.alltotalcnt.values()) != sum(self.allpassedcnt.values()) + return len(self.errlist) > 0 # End of Class Validator def main() -> None: - parser = argparse.ArgumentParser(description="Launch metric value validation") - - parser.add_argument("-rule", help="Base validation rule file", required=True) - parser.add_argument("-output_dir", help="Path for validator output file, report file", required=True) - parser.add_argument("-debug", help="Debug run, save intermediate data to files", action="store_true", default=False) - parser.add_argument("-wl", help="Workload to run while data collection", default="true") + parser = argparse.ArgumentParser( + description="Launch metric value validation") + + parser.add_argument( + "-rule", help="Base validation rule file", required=True) + parser.add_argument( + "-output_dir", help="Path for validator output file, report file", required=True) + parser.add_argument("-debug", help="Debug run, save intermediate data to files", + action="store_true", default=False) + parser.add_argument( + "-wl", help="Workload to run while data collection", default="true") parser.add_argument("-m", help="Metric list to validate", default="") args = parser.parse_args() outpath = Path(args.output_dir) @@ -559,8 +579,8 @@ def main() -> None: datafile = Path.joinpath(outpath, 'perf_data.json') validator = Validator(args.rule, reportf, debug=args.debug, - datafname=datafile, fullrulefname=fullrule, workload=args.wl, - metrics=args.m) + datafname=datafile, fullrulefname=fullrule, workload=args.wl, + metrics=args.m) ret = validator.test() return ret @@ -569,6 +589,3 @@ def main() -> None: if __name__ == "__main__": import sys sys.exit(main()) - - - diff --git a/tools/perf/tests/shell/stat_metrics_values.sh b/tools/perf/tests/shell/stat_metrics_values.sh index 7ca172599aa6..279f19c5919a 100755 --- a/tools/perf/tests/shell/stat_metrics_values.sh +++ b/tools/perf/tests/shell/stat_metrics_values.sh @@ -19,6 +19,8 @@ echo "Output will be stored in: $tmpdir" $PYTHON $pythonvalidator -rule $rulefile -output_dir $tmpdir -wl "${workload}" ret=$? rm -rf $tmpdir - +if [ $ret -ne 0 ]; then + echo "Metric validation return with erros. Please check metrics reported with errors." +fi exit $ret -- cgit v1.2.3 From fd7b8e8fb20f51d60dfee7792806548f3c6a4c2c Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 31 Jan 2024 05:49:40 -0800 Subject: perf parse-events: Print all errors Prior to this patch the first and the last error encountered during parsing are printed. To see other errors verbose needs enabling. Unfortunately this can drop useful errors, in particular on terms. This patch changes the errors so that instead of the first and last all errors are recorded and printed, the underlying data structure is changed to a list. Before: ``` $ perf stat -e 'slots/edge=2/' true event syntax error: 'slots/edge=2/' \___ Bad event or PMU Unable to find PMU or event on a PMU of 'slots' Initial error: event syntax error: 'slots/edge=2/' \___ Cannot find PMU `slots'. Missing kernel support? Run 'perf list' for a list of valid events Usage: perf stat [] [] -e, --event event selector. use 'perf list' to list available events ``` After: ``` $ perf stat -e 'slots/edge=2/' true event syntax error: 'slots/edge=2/' \___ Bad event or PMU Unable to find PMU or event on a PMU of 'slots' event syntax error: 'slots/edge=2/' \___ value too big for format (edge), maximum is 1 event syntax error: 'slots/edge=2/' \___ Cannot find PMU `slots'. Missing kernel support? Run 'perf list' for a list of valid events Usage: perf stat [] [] -e, --event event selector. use 'perf list' to list available events ``` Signed-off-by: Ian Rogers Reviewed-by: James Clark Cc: Mark Rutland Cc: tchen168@asu.edu Cc: Kan Liang Cc: Michael Petlan Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240131134940.593788-3-irogers@google.com --- tools/perf/arch/x86/tests/hybrid.c | 5 +-- tools/perf/tests/expand-cgroup.c | 3 +- tools/perf/tests/parse-events.c | 9 ++-- tools/perf/util/parse-events.c | 92 +++++++++++++++++++++++--------------- tools/perf/util/parse-events.h | 14 +++--- tools/perf/util/parse-events.y | 2 - 6 files changed, 67 insertions(+), 58 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/arch/x86/tests/hybrid.c b/tools/perf/arch/x86/tests/hybrid.c index 40f5d17fedab..e221ea104174 100644 --- a/tools/perf/arch/x86/tests/hybrid.c +++ b/tools/perf/arch/x86/tests/hybrid.c @@ -259,11 +259,10 @@ static int test_event(const struct evlist_test *e) parse_events_error__init(&err); ret = parse_events(evlist, e->name, &err); if (ret) { - pr_debug("failed to parse event '%s', err %d, str '%s'\n", - e->name, ret, err.str); + pr_debug("failed to parse event '%s', err %d\n", e->name, ret); parse_events_error__print(&err, e->name); ret = TEST_FAIL; - if (strstr(err.str, "can't access trace events")) + if (parse_events_error__contains(&err, "can't access trace events")) ret = TEST_SKIP; } else { ret = e->check(evlist); diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c index 9c1a1f18db75..31966ff856f8 100644 --- a/tools/perf/tests/expand-cgroup.c +++ b/tools/perf/tests/expand-cgroup.c @@ -127,8 +127,7 @@ static int expand_group_events(void) parse_events_error__init(&err); ret = parse_events(evlist, event_str, &err); if (ret < 0) { - pr_debug("failed to parse event '%s', err %d, str '%s'\n", - event_str, ret, err.str); + pr_debug("failed to parse event '%s', err %d\n", event_str, ret); parse_events_error__print(&err, event_str); goto out; } diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index fbdf710d5eea..feb5727584d1 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -2506,11 +2506,10 @@ static int test_event(const struct evlist_test *e) parse_events_error__init(&err); ret = parse_events(evlist, e->name, &err); if (ret) { - pr_debug("failed to parse event '%s', err %d, str '%s'\n", - e->name, ret, err.str); + pr_debug("failed to parse event '%s', err %d\n", e->name, ret); parse_events_error__print(&err, e->name); ret = TEST_FAIL; - if (err.str && strstr(err.str, "can't access trace events")) + if (parse_events_error__contains(&err, "can't access trace events")) ret = TEST_SKIP; } else { ret = e->check(evlist); @@ -2535,8 +2534,8 @@ static int test_event_fake_pmu(const char *str) ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err, &perf_pmu__fake, /*warn_if_reordered=*/true); if (ret) { - pr_debug("failed to parse event '%s', err %d, str '%s'\n", - str, ret, err.str); + pr_debug("failed to parse event '%s', err %d\n", + str, ret); parse_events_error__print(&err, str); } diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 66eabcea4242..6f8b0fa17689 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2181,50 +2181,53 @@ int parse_event(struct evlist *evlist, const char *str) return ret; } +struct parse_events_error_entry { + /** @list: The list the error is part of. */ + struct list_head list; + /** @idx: index in the parsed string */ + int idx; + /** @str: string to display at the index */ + char *str; + /** @help: optional help string */ + char *help; +}; + void parse_events_error__init(struct parse_events_error *err) { - bzero(err, sizeof(*err)); + INIT_LIST_HEAD(&err->list); } void parse_events_error__exit(struct parse_events_error *err) { - zfree(&err->str); - zfree(&err->help); - zfree(&err->first_str); - zfree(&err->first_help); + struct parse_events_error_entry *pos, *tmp; + + list_for_each_entry_safe(pos, tmp, &err->list, list) { + zfree(&pos->str); + zfree(&pos->help); + list_del_init(&pos->list); + free(pos); + } } void parse_events_error__handle(struct parse_events_error *err, int idx, char *str, char *help) { + struct parse_events_error_entry *entry; + if (WARN(!str || !err, "WARNING: failed to provide error string or struct\n")) goto out_free; - switch (err->num_errors) { - case 0: - err->idx = idx; - err->str = str; - err->help = help; - break; - case 1: - err->first_idx = err->idx; - err->idx = idx; - err->first_str = err->str; - err->str = str; - err->first_help = err->help; - err->help = help; - break; - default: - pr_debug("Multiple errors dropping message: %s (%s)\n", - err->str, err->help ?: ""); - free(err->str); - err->str = str; - free(err->help); - err->help = help; - break; + + entry = zalloc(sizeof(*entry)); + if (!entry) { + pr_err("Failed to allocate memory for event parsing error: %s (%s)\n", + str, help ?: ""); + goto out_free; } - err->num_errors++; + entry->idx = idx; + entry->str = str; + entry->help = help; + list_add(&entry->list, &err->list); return; - out_free: free(str); free(help); @@ -2294,19 +2297,34 @@ static void __parse_events_error__print(int err_idx, const char *err_str, } } -void parse_events_error__print(struct parse_events_error *err, +void parse_events_error__print(const struct parse_events_error *err, const char *event) { - if (!err->num_errors) - return; + struct parse_events_error_entry *pos; + bool first = true; + + list_for_each_entry(pos, &err->list, list) { + if (!first) + fputs("\n", stderr); + __parse_events_error__print(pos->idx, pos->str, pos->help, event); + first = false; + } +} - __parse_events_error__print(err->idx, err->str, err->help, event); +/* + * In the list of errors err, do any of the error strings (str) contain the + * given needle string? + */ +bool parse_events_error__contains(const struct parse_events_error *err, + const char *needle) +{ + struct parse_events_error_entry *pos; - if (err->num_errors > 1) { - fputs("\nInitial error:\n", stderr); - __parse_events_error__print(err->first_idx, err->first_str, - err->first_help, event); + list_for_each_entry(pos, &err->list, list) { + if (strstr(pos->str, needle) != NULL) + return true; } + return false; } #undef MAX_WIDTH diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 63c0a36a4bf1..809359e8544e 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -130,13 +130,8 @@ struct parse_events_term { }; struct parse_events_error { - int num_errors; /* number of errors encountered */ - int idx; /* index in the parsed string */ - char *str; /* string to display at the index */ - char *help; /* optional help string */ - int first_idx;/* as above, but for the first encountered error */ - char *first_str; - char *first_help; + /** @list: The head of a list of errors. */ + struct list_head list; }; /* A wrapper around a list of terms for the sake of better type safety. */ @@ -247,9 +242,10 @@ void parse_events_error__init(struct parse_events_error *err); void parse_events_error__exit(struct parse_events_error *err); void parse_events_error__handle(struct parse_events_error *err, int idx, char *str, char *help); -void parse_events_error__print(struct parse_events_error *err, +void parse_events_error__print(const struct parse_events_error *err, const char *event); - +bool parse_events_error__contains(const struct parse_events_error *err, + const char *needle); #ifdef HAVE_LIBELF_SUPPORT /* * If the probe point starts with '%', diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index de098caf0c1c..d70f5d84af92 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -536,8 +536,6 @@ tracepoint_name opt_event_config list = alloc_list(); if (!list) YYNOMEM; - if (error) - error->idx = @1.first_column; err = parse_events_add_tracepoint(list, &parse_state->idx, $1.sys, $1.event, error, $2, &@1); -- cgit v1.2.3 From b8db070f389c902f48e83ee7a94952e9557199e8 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 31 Jan 2024 12:14:29 -0800 Subject: perf jevents: Drop or simplify small integer values Prior to this patch '0' would be dropped as the config values default to 0. Some json values are hex and the string '0' wouldn't match '0x0' as zero. Add a more robust is_zero test to drop these event terms. When encoding numbers as hex, if the number is between 0 and 9 inclusive then don't add a 0x prefix. Update test expectations for these changes. On x86 this reduces the event/metric C string by 58,411 bytes. Signed-off-by: Ian Rogers Reviewed-by: Kan Liang Cc: Edward Baker Cc: Perry Taylor Cc: Weilin Wang Cc: John Garry Cc: Jing Zhang Cc: Kajol Jain Cc: Michael Petlan Cc: Veronika Molnarova Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240131201429.792138-1-irogers@google.com --- tools/perf/pmu-events/jevents.py | 23 ++++++++++++++++++++--- tools/perf/tests/pmu-events.c | 22 +++++++++++----------- 2 files changed, 31 insertions(+), 14 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py index 53ab050c8fa4..2c7e5d61ce92 100755 --- a/tools/perf/pmu-events/jevents.py +++ b/tools/perf/pmu-events/jevents.py @@ -203,7 +203,7 @@ class JsonEvent: def llx(x: int) -> str: """Convert an int to a string similar to a printf modifier of %#llx.""" - return '0' if x == 0 else hex(x) + return str(x) if x >= 0 and x < 10 else hex(x) def fixdesc(s: str) -> str: """Fix formatting issue for the desc string.""" @@ -294,6 +294,23 @@ class JsonEvent: } return table[unit] if unit in table else f'uncore_{unit.lower()}' + def is_zero(val: str) -> bool: + try: + if val.startswith('0x'): + return int(val, 16) == 0 + else: + return int(val) == 0 + except e: + return False + + def canonicalize_value(val: str) -> str: + try: + if val.startswith('0x'): + return llx(int(val, 16)) + return str(int(val)) + except e: + return val + eventcode = 0 if 'EventCode' in jd: eventcode = int(jd['EventCode'].split(',', 1)[0], 0) @@ -358,8 +375,8 @@ class JsonEvent: ('RdWrMask', 'rdwrmask='), ] for key, value in event_fields: - if key in jd and jd[key] != '0': - event += ',' + value + jd[key] + if key in jd and not is_zero(jd[key]): + event += f',{value}{canonicalize_value(jd[key])}' if filter: event += f',{filter}' if msr: diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c index a56d32905743..47a7c3277540 100644 --- a/tools/perf/tests/pmu-events.c +++ b/tools/perf/tests/pmu-events.c @@ -70,7 +70,7 @@ static const struct perf_pmu_test_event segment_reg_loads_any = { .event = { .pmu = "default_core", .name = "segment_reg_loads.any", - .event = "event=0x6,period=200000,umask=0x80", + .event = "event=6,period=200000,umask=0x80", .desc = "Number of segment register loads", .topic = "other", }, @@ -82,7 +82,7 @@ static const struct perf_pmu_test_event dispatch_blocked_any = { .event = { .pmu = "default_core", .name = "dispatch_blocked.any", - .event = "event=0x9,period=200000,umask=0x20", + .event = "event=9,period=200000,umask=0x20", .desc = "Memory cluster signals to block micro-op dispatch for any reason", .topic = "other", }, @@ -94,11 +94,11 @@ static const struct perf_pmu_test_event eist_trans = { .event = { .pmu = "default_core", .name = "eist_trans", - .event = "event=0x3a,period=200000,umask=0x0", + .event = "event=0x3a,period=200000", .desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions", .topic = "other", }, - .alias_str = "event=0x3a,period=0x30d40,umask=0", + .alias_str = "event=0x3a,period=0x30d40", .alias_long_desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions", }; @@ -128,7 +128,7 @@ static const struct perf_pmu_test_event *core_events[] = { static const struct perf_pmu_test_event uncore_hisi_ddrc_flux_wcmd = { .event = { .name = "uncore_hisi_ddrc.flux_wcmd", - .event = "event=0x2", + .event = "event=2", .desc = "DDRC write commands", .topic = "uncore", .long_desc = "DDRC write commands", @@ -156,13 +156,13 @@ static const struct perf_pmu_test_event unc_cbo_xsnp_response_miss_eviction = { static const struct perf_pmu_test_event uncore_hyphen = { .event = { .name = "event-hyphen", - .event = "event=0xe0,umask=0x00", + .event = "event=0xe0", .desc = "UNC_CBO_HYPHEN", .topic = "uncore", .long_desc = "UNC_CBO_HYPHEN", .pmu = "uncore_cbox", }, - .alias_str = "event=0xe0,umask=0", + .alias_str = "event=0xe0", .alias_long_desc = "UNC_CBO_HYPHEN", .matching_pmu = "uncore_cbox_0", }; @@ -170,13 +170,13 @@ static const struct perf_pmu_test_event uncore_hyphen = { static const struct perf_pmu_test_event uncore_two_hyph = { .event = { .name = "event-two-hyph", - .event = "event=0xc0,umask=0x00", + .event = "event=0xc0", .desc = "UNC_CBO_TWO_HYPH", .topic = "uncore", .long_desc = "UNC_CBO_TWO_HYPH", .pmu = "uncore_cbox", }, - .alias_str = "event=0xc0,umask=0", + .alias_str = "event=0xc0", .alias_long_desc = "UNC_CBO_TWO_HYPH", .matching_pmu = "uncore_cbox_0", }; @@ -184,7 +184,7 @@ static const struct perf_pmu_test_event uncore_two_hyph = { static const struct perf_pmu_test_event uncore_hisi_l3c_rd_hit_cpipe = { .event = { .name = "uncore_hisi_l3c.rd_hit_cpipe", - .event = "event=0x7", + .event = "event=7", .desc = "Total read hits", .topic = "uncore", .long_desc = "Total read hits", @@ -265,7 +265,7 @@ static const struct perf_pmu_test_event sys_ccn_pmu_read_cycles = { static const struct perf_pmu_test_event sys_cmn_pmu_hnf_cache_miss = { .event = { .name = "sys_cmn_pmu.hnf_cache_miss", - .event = "eventid=0x1,type=0x5", + .event = "eventid=1,type=5", .desc = "Counts total cache misses in first lookup result (high priority)", .topic = "uncore", .pmu = "uncore_sys_cmn_pmu", -- cgit v1.2.3 From 5f70c6c559908984ea93d61a62108b2aff017a99 Mon Sep 17 00:00:00 2001 From: Yicong Yang Date: Wed, 7 Feb 2024 17:12:22 +0800 Subject: perf test: Skip metric w/o event name on arm64 in stat STD output linter stat+std_output.sh test fails on my arm64 machine: [root@localhost shell]# ./stat+std_output.sh Checking STD output: no args Unknown event name in TopDownL1 # 0.18 retiring [root@localhost shell]# ./stat+std_output.sh Checking STD output: no args [Success] Checking STD output: system wide [Success] Checking STD output: interval [Success] Checking STD output: per thread Unknown event name in tmux: server-1114960 # 0.41 frontend_bound When no args specified `perf stat` will add TopdownL1 metric group and the output will be like: [root@localhost shell]# perf stat -- stress-ng --vm 1 --timeout 1 stress-ng: info: [3351733] setting to a 1 second run per stressor stress-ng: info: [3351733] dispatching hogs: 1 vm stress-ng: info: [3351733] successful run completed in 1.02s Performance counter stats for 'stress-ng --vm 1 --timeout 1': 1,037.71 msec task-clock # 1.000 CPUs utilized 13 context-switches # 12.528 /sec 1 cpu-migrations # 0.964 /sec 67,544 page-faults # 65.090 K/sec 2,691,932,561 cycles # 2.594 GHz (74.56%) 6,571,333,653 instructions # 2.44 insn per cycle (74.92%) 521,863,142 branches # 502.901 M/sec (75.21%) 425,879 branch-misses # 0.08% of all branches (87.57%) TopDownL1 # 0.61 retiring (87.67%) # 0.03 frontend_bound (87.67%) # 0.02 bad_speculation (87.67%) # 0.34 backend_bound (74.61%) 1.038138390 seconds time elapsed 0.844849000 seconds user 0.189053000 seconds sys Metrics in group TopDownL1 don't have event name on arm64 but are not listed in the $skip_metric list which they should be listed. Add them to the skip list as what does for x86 platforms in [1]. [1] commit 4d60e83dfcee ("perf test: Skip metrics w/o event name in stat STD output linter") Signed-off-by: Yicong Yang Reviewed-by: Ian Rogers Cc: linuxarm@huawei.com Cc: kan.liang@linux.intel.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240207091222.54096-1-yangyicong@huawei.com --- tools/perf/tests/shell/stat+std_output.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/stat+std_output.sh b/tools/perf/tests/shell/stat+std_output.sh index 4fcdd1a9142c..f46a0c9908c0 100755 --- a/tools/perf/tests/shell/stat+std_output.sh +++ b/tools/perf/tests/shell/stat+std_output.sh @@ -13,7 +13,7 @@ stat_output=$(mktemp /tmp/__perf_test.stat_output.std.XXXXX) event_name=(cpu-clock task-clock context-switches cpu-migrations page-faults stalled-cycles-frontend stalled-cycles-backend cycles instructions branches branch-misses) event_metric=("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "frontend cycles idle" "backend cycles idle" "GHz" "insn per cycle" "/sec" "of all branches") -skip_metric=("stalled cycles per insn" "tma_") +skip_metric=("stalled cycles per insn" "tma_" "retiring" "frontend_bound" "bad_speculation" "backend_bound") cleanup() { rm -f "${stat_output}" -- cgit v1.2.3 From cbc917a1b03bce85f385c1e640c9dcb02ffb9ab0 Mon Sep 17 00:00:00 2001 From: Yicong Yang Date: Thu, 8 Feb 2024 10:40:26 +0800 Subject: perf stat: Support per-cluster aggregation Some platforms have 'cluster' topology and CPUs in the cluster will share resources like L3 Cache Tag (for HiSilicon Kunpeng SoC) or L2 cache (for Intel Jacobsville). Currently parsing and building cluster topology have been supported since [1]. perf stat has already supported aggregation for other topologies like die or socket, etc. It'll be useful to aggregate per-cluster to find problems like L3T bandwidth contention. This patch add support for "--per-cluster" option for per-cluster aggregation. Also update the docs and related test. The output will be like: [root@localhost tmp]# perf stat -a -e LLC-load --per-cluster -- sleep 5 Performance counter stats for 'system wide': S56-D0-CLS158 4 1,321,521,570 LLC-load S56-D0-CLS594 4 794,211,453 LLC-load S56-D0-CLS1030 4 41,623 LLC-load S56-D0-CLS1466 4 41,646 LLC-load S56-D0-CLS1902 4 16,863 LLC-load S56-D0-CLS2338 4 15,721 LLC-load S56-D0-CLS2774 4 22,671 LLC-load [...] On a legacy system without cluster or cluster support, the output will be look like: [root@localhost perf]# perf stat -a -e cycles --per-cluster -- sleep 1 Performance counter stats for 'system wide': S56-D0-CLS0 64 18,011,485 cycles S7182-D0-CLS0 64 16,548,835 cycles Note that this patch doesn't mix the cluster information in the outputs of --per-core to avoid breaking any tools/scripts using it. Note that perf recently supports "--per-cache" aggregation, but it's not the same with the cluster although cluster CPUs may share some cache resources. For example on my machine all clusters within a die share the same L3 cache: $ cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list 0-31 $ cat /sys/devices/system/cpu/cpu0/topology/cluster_cpus_list 0-3 [1] commit c5e22feffdd7 ("topology: Represent clusters of CPUs within a die") Tested-by: Jie Zhan Reviewed-by: Tim Chen Reviewed-by: Ian Rogers Signed-off-by: Yicong Yang Cc: james.clark@arm.com Cc: 21cnbao@gmail.com Cc: prime.zeng@hisilicon.com Cc: Jonathan.Cameron@huawei.com Cc: fanghao11@huawei.com Cc: linuxarm@huawei.com Cc: tim.c.chen@intel.com Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240208024026.2691-1-yangyicong@huawei.com --- tools/perf/Documentation/perf-stat.txt | 11 +++++ tools/perf/builtin-stat.c | 52 ++++++++++++++++++++-- .../perf/tests/shell/lib/perf_json_output_lint.py | 4 +- tools/perf/tests/shell/lib/stat_output.sh | 12 +++++ tools/perf/tests/shell/stat+csv_output.sh | 2 + tools/perf/tests/shell/stat+json_output.sh | 13 ++++++ tools/perf/tests/shell/stat+std_output.sh | 2 + tools/perf/util/cpumap.c | 33 +++++++++++++- tools/perf/util/cpumap.h | 19 ++++++-- tools/perf/util/env.h | 1 + tools/perf/util/stat-display.c | 13 ++++++ tools/perf/util/stat.h | 1 + 12 files changed, 154 insertions(+), 9 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt index 5af2e432b54f..29756a87ab6f 100644 --- a/tools/perf/Documentation/perf-stat.txt +++ b/tools/perf/Documentation/perf-stat.txt @@ -308,6 +308,14 @@ use --per-die in addition to -a. (system-wide). The output includes the die number and the number of online processors on that die. This is useful to gauge the amount of aggregation. +--per-cluster:: +Aggregate counts per processor cluster for system-wide mode measurement. This +is a useful mode to detect imbalance between clusters. To enable this mode, +use --per-cluster in addition to -a. (system-wide). The output includes the +cluster number and the number of online processors on that cluster. This is +useful to gauge the amount of aggregation. The information of cluster ID and +related CPUs can be gotten from /sys/devices/system/cpu/cpuX/topology/cluster_{id, cpus}. + --per-cache:: Aggregate counts per cache instance for system-wide mode measurements. By default, the aggregation happens for the cache level at the highest index @@ -396,6 +404,9 @@ Aggregate counts per processor socket for system-wide mode measurements. --per-die:: Aggregate counts per processor die for system-wide mode measurements. +--per-cluster:: +Aggregate counts perf processor cluster for system-wide mode measurements. + --per-cache:: Aggregate counts per cache instance for system-wide mode measurements. By default, the aggregation happens for the cache level at the highest index diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 5fe9abc6a524..6bba1a89d030 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -1238,6 +1238,8 @@ static struct option stat_options[] = { "aggregate counts per processor socket", AGGR_SOCKET), OPT_SET_UINT(0, "per-die", &stat_config.aggr_mode, "aggregate counts per processor die", AGGR_DIE), + OPT_SET_UINT(0, "per-cluster", &stat_config.aggr_mode, + "aggregate counts per processor cluster", AGGR_CLUSTER), OPT_CALLBACK_OPTARG(0, "per-cache", &stat_config.aggr_mode, &stat_config.aggr_level, "cache level", "aggregate count at this cache level (Default: LLC)", parse_cache_level), @@ -1428,6 +1430,7 @@ static struct aggr_cpu_id aggr_cpu_id__cache(struct perf_cpu cpu, void *data) static const char *const aggr_mode__string[] = { [AGGR_CORE] = "core", [AGGR_CACHE] = "cache", + [AGGR_CLUSTER] = "cluster", [AGGR_DIE] = "die", [AGGR_GLOBAL] = "global", [AGGR_NODE] = "node", @@ -1455,6 +1458,12 @@ static struct aggr_cpu_id perf_stat__get_cache_id(struct perf_stat_config *confi return aggr_cpu_id__cache(cpu, /*data=*/NULL); } +static struct aggr_cpu_id perf_stat__get_cluster(struct perf_stat_config *config __maybe_unused, + struct perf_cpu cpu) +{ + return aggr_cpu_id__cluster(cpu, /*data=*/NULL); +} + static struct aggr_cpu_id perf_stat__get_core(struct perf_stat_config *config __maybe_unused, struct perf_cpu cpu) { @@ -1507,6 +1516,12 @@ static struct aggr_cpu_id perf_stat__get_die_cached(struct perf_stat_config *con return perf_stat__get_aggr(config, perf_stat__get_die, cpu); } +static struct aggr_cpu_id perf_stat__get_cluster_cached(struct perf_stat_config *config, + struct perf_cpu cpu) +{ + return perf_stat__get_aggr(config, perf_stat__get_cluster, cpu); +} + static struct aggr_cpu_id perf_stat__get_cache_id_cached(struct perf_stat_config *config, struct perf_cpu cpu) { @@ -1544,6 +1559,8 @@ static aggr_cpu_id_get_t aggr_mode__get_aggr(enum aggr_mode aggr_mode) return aggr_cpu_id__socket; case AGGR_DIE: return aggr_cpu_id__die; + case AGGR_CLUSTER: + return aggr_cpu_id__cluster; case AGGR_CACHE: return aggr_cpu_id__cache; case AGGR_CORE: @@ -1569,6 +1586,8 @@ static aggr_get_id_t aggr_mode__get_id(enum aggr_mode aggr_mode) return perf_stat__get_socket_cached; case AGGR_DIE: return perf_stat__get_die_cached; + case AGGR_CLUSTER: + return perf_stat__get_cluster_cached; case AGGR_CACHE: return perf_stat__get_cache_id_cached; case AGGR_CORE: @@ -1737,6 +1756,21 @@ static struct aggr_cpu_id perf_env__get_cache_aggr_by_cpu(struct perf_cpu cpu, return id; } +static struct aggr_cpu_id perf_env__get_cluster_aggr_by_cpu(struct perf_cpu cpu, + void *data) +{ + struct perf_env *env = data; + struct aggr_cpu_id id = aggr_cpu_id__empty(); + + if (cpu.cpu != -1) { + id.socket = env->cpu[cpu.cpu].socket_id; + id.die = env->cpu[cpu.cpu].die_id; + id.cluster = env->cpu[cpu.cpu].cluster_id; + } + + return id; +} + static struct aggr_cpu_id perf_env__get_core_aggr_by_cpu(struct perf_cpu cpu, void *data) { struct perf_env *env = data; @@ -1744,12 +1778,12 @@ static struct aggr_cpu_id perf_env__get_core_aggr_by_cpu(struct perf_cpu cpu, vo if (cpu.cpu != -1) { /* - * core_id is relative to socket and die, - * we need a global id. So we set - * socket, die id and core id + * core_id is relative to socket, die and cluster, we need a + * global id. So we set socket, die id, cluster id and core id. */ id.socket = env->cpu[cpu.cpu].socket_id; id.die = env->cpu[cpu.cpu].die_id; + id.cluster = env->cpu[cpu.cpu].cluster_id; id.core = env->cpu[cpu.cpu].core_id; } @@ -1805,6 +1839,12 @@ static struct aggr_cpu_id perf_stat__get_die_file(struct perf_stat_config *confi return perf_env__get_die_aggr_by_cpu(cpu, &perf_stat.session->header.env); } +static struct aggr_cpu_id perf_stat__get_cluster_file(struct perf_stat_config *config __maybe_unused, + struct perf_cpu cpu) +{ + return perf_env__get_cluster_aggr_by_cpu(cpu, &perf_stat.session->header.env); +} + static struct aggr_cpu_id perf_stat__get_cache_file(struct perf_stat_config *config __maybe_unused, struct perf_cpu cpu) { @@ -1842,6 +1882,8 @@ static aggr_cpu_id_get_t aggr_mode__get_aggr_file(enum aggr_mode aggr_mode) return perf_env__get_socket_aggr_by_cpu; case AGGR_DIE: return perf_env__get_die_aggr_by_cpu; + case AGGR_CLUSTER: + return perf_env__get_cluster_aggr_by_cpu; case AGGR_CACHE: return perf_env__get_cache_aggr_by_cpu; case AGGR_CORE: @@ -1867,6 +1909,8 @@ static aggr_get_id_t aggr_mode__get_id_file(enum aggr_mode aggr_mode) return perf_stat__get_socket_file; case AGGR_DIE: return perf_stat__get_die_file; + case AGGR_CLUSTER: + return perf_stat__get_cluster_file; case AGGR_CACHE: return perf_stat__get_cache_file; case AGGR_CORE: @@ -2398,6 +2442,8 @@ static int __cmd_report(int argc, const char **argv) "aggregate counts per processor socket", AGGR_SOCKET), OPT_SET_UINT(0, "per-die", &perf_stat.aggr_mode, "aggregate counts per processor die", AGGR_DIE), + OPT_SET_UINT(0, "per-cluster", &perf_stat.aggr_mode, + "aggregate counts perf processor cluster", AGGR_CLUSTER), OPT_CALLBACK_OPTARG(0, "per-cache", &perf_stat.aggr_mode, &perf_stat.aggr_level, "cache level", "aggregate count at this cache level (Default: LLC)", diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py index ea55d5ea1ced..abc1fd737782 100644 --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py @@ -15,6 +15,7 @@ ap.add_argument('--event', action='store_true') ap.add_argument('--per-core', action='store_true') ap.add_argument('--per-thread', action='store_true') ap.add_argument('--per-cache', action='store_true') +ap.add_argument('--per-cluster', action='store_true') ap.add_argument('--per-die', action='store_true') ap.add_argument('--per-node', action='store_true') ap.add_argument('--per-socket', action='store_true') @@ -49,6 +50,7 @@ def check_json_output(expected_items): 'cgroup': lambda x: True, 'cpu': lambda x: isint(x), 'cache': lambda x: True, + 'cluster': lambda x: True, 'die': lambda x: True, 'event': lambda x: True, 'event-runtime': lambda x: isfloat(x), @@ -88,7 +90,7 @@ try: expected_items = 7 elif args.interval or args.per_thread or args.system_wide_no_aggr: expected_items = 8 - elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache: + elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cluster or args.per_cache: expected_items = 9 else: # If no option is specified, don't check the number of items. diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh index 3cc158a64326..c81d6a9f7983 100644 --- a/tools/perf/tests/shell/lib/stat_output.sh +++ b/tools/perf/tests/shell/lib/stat_output.sh @@ -97,6 +97,18 @@ check_per_cache_instance() echo "[Success]" } +check_per_cluster() +{ + echo -n "Checking $1 output: per cluster " + if ParanoidAndNotRoot 0 + then + echo "[Skip] paranoid and not root" + return + fi + perf stat --per-cluster -a $2 true + echo "[Success]" +} + check_per_die() { echo -n "Checking $1 output: per die " diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh index f1818fa6d9ce..fc2d8cc6e5e0 100755 --- a/tools/perf/tests/shell/stat+csv_output.sh +++ b/tools/perf/tests/shell/stat+csv_output.sh @@ -42,6 +42,7 @@ function commachecker() ;; "--per-socket") exp=8 ;; "--per-node") exp=8 ;; "--per-die") exp=8 + ;; "--per-cluster") exp=8 ;; "--per-cache") exp=8 esac @@ -79,6 +80,7 @@ then check_system_wide_no_aggr "CSV" "$perf_cmd" check_per_core "CSV" "$perf_cmd" check_per_cache_instance "CSV" "$perf_cmd" + check_per_cluster "CSV" "$perf_cmd" check_per_die "CSV" "$perf_cmd" check_per_socket "CSV" "$perf_cmd" else diff --git a/tools/perf/tests/shell/stat+json_output.sh b/tools/perf/tests/shell/stat+json_output.sh index 3bc900533a5d..2b9c6212dffc 100755 --- a/tools/perf/tests/shell/stat+json_output.sh +++ b/tools/perf/tests/shell/stat+json_output.sh @@ -122,6 +122,18 @@ check_per_cache_instance() echo "[Success]" } +check_per_cluster() +{ + echo -n "Checking json output: per cluster " + if ParanoidAndNotRoot 0 + then + echo "[Skip] paranoia and not root" + return + fi + perf stat -j --per-cluster -a true 2>&1 | $PYTHON $pythonchecker --per-cluster + echo "[Success]" +} + check_per_die() { echo -n "Checking json output: per die " @@ -200,6 +212,7 @@ then check_system_wide_no_aggr check_per_core check_per_cache_instance + check_per_cluster check_per_die check_per_socket else diff --git a/tools/perf/tests/shell/stat+std_output.sh b/tools/perf/tests/shell/stat+std_output.sh index f46a0c9908c0..cbf2894b2c84 100755 --- a/tools/perf/tests/shell/stat+std_output.sh +++ b/tools/perf/tests/shell/stat+std_output.sh @@ -40,6 +40,7 @@ function commachecker() ;; "--per-node") prefix=3 ;; "--per-die") prefix=3 ;; "--per-cache") prefix=3 + ;; "--per-cluster") prefix=3 esac while read line @@ -99,6 +100,7 @@ then check_system_wide_no_aggr "STD" "$perf_cmd" check_per_core "STD" "$perf_cmd" check_per_cache_instance "STD" "$perf_cmd" + check_per_cluster "STD" "$perf_cmd" check_per_die "STD" "$perf_cmd" check_per_socket "STD" "$perf_cmd" else diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c index 0581ee0fa5f2..356e30c42cd8 100644 --- a/tools/perf/util/cpumap.c +++ b/tools/perf/util/cpumap.c @@ -222,6 +222,8 @@ static int aggr_cpu_id__cmp(const void *a_pointer, const void *b_pointer) return a->socket - b->socket; else if (a->die != b->die) return a->die - b->die; + else if (a->cluster != b->cluster) + return a->cluster - b->cluster; else if (a->cache_lvl != b->cache_lvl) return a->cache_lvl - b->cache_lvl; else if (a->cache != b->cache) @@ -309,6 +311,30 @@ struct aggr_cpu_id aggr_cpu_id__die(struct perf_cpu cpu, void *data) return id; } +int cpu__get_cluster_id(struct perf_cpu cpu) +{ + int value, ret = cpu__get_topology_int(cpu.cpu, "cluster_id", &value); + + return ret ?: value; +} + +struct aggr_cpu_id aggr_cpu_id__cluster(struct perf_cpu cpu, void *data) +{ + int cluster = cpu__get_cluster_id(cpu); + struct aggr_cpu_id id; + + /* There is no cluster_id on legacy system. */ + if (cluster == -1) + cluster = 0; + + id = aggr_cpu_id__die(cpu, data); + if (aggr_cpu_id__is_empty(&id)) + return id; + + id.cluster = cluster; + return id; +} + int cpu__get_core_id(struct perf_cpu cpu) { int value, ret = cpu__get_topology_int(cpu.cpu, "core_id", &value); @@ -320,8 +346,8 @@ struct aggr_cpu_id aggr_cpu_id__core(struct perf_cpu cpu, void *data) struct aggr_cpu_id id; int core = cpu__get_core_id(cpu); - /* aggr_cpu_id__die returns a struct with socket and die set. */ - id = aggr_cpu_id__die(cpu, data); + /* aggr_cpu_id__die returns a struct with socket die, and cluster set. */ + id = aggr_cpu_id__cluster(cpu, data); if (aggr_cpu_id__is_empty(&id)) return id; @@ -683,6 +709,7 @@ bool aggr_cpu_id__equal(const struct aggr_cpu_id *a, const struct aggr_cpu_id *b a->node == b->node && a->socket == b->socket && a->die == b->die && + a->cluster == b->cluster && a->cache_lvl == b->cache_lvl && a->cache == b->cache && a->core == b->core && @@ -695,6 +722,7 @@ bool aggr_cpu_id__is_empty(const struct aggr_cpu_id *a) a->node == -1 && a->socket == -1 && a->die == -1 && + a->cluster == -1 && a->cache_lvl == -1 && a->cache == -1 && a->core == -1 && @@ -708,6 +736,7 @@ struct aggr_cpu_id aggr_cpu_id__empty(void) .node = -1, .socket = -1, .die = -1, + .cluster = -1, .cache_lvl = -1, .cache = -1, .core = -1, diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h index 9df2aeb34d3d..26cf76c693f5 100644 --- a/tools/perf/util/cpumap.h +++ b/tools/perf/util/cpumap.h @@ -20,6 +20,8 @@ struct aggr_cpu_id { int socket; /** The die id as read from /sys/devices/system/cpu/cpuX/topology/die_id. */ int die; + /** The cluster id as read from /sys/devices/system/cpu/cpuX/topology/cluster_id */ + int cluster; /** The cache level as read from /sys/devices/system/cpu/cpuX/cache/indexY/level */ int cache_lvl; /** @@ -86,6 +88,11 @@ int cpu__get_socket_id(struct perf_cpu cpu); * /sys/devices/system/cpu/cpuX/topology/die_id for the given CPU. */ int cpu__get_die_id(struct perf_cpu cpu); +/** + * cpu__get_cluster_id - Returns the cluster id as read from + * /sys/devices/system/cpu/cpuX/topology/cluster_id for the given CPU + */ +int cpu__get_cluster_id(struct perf_cpu cpu); /** * cpu__get_core_id - Returns the core id as read from * /sys/devices/system/cpu/cpuX/topology/core_id for the given CPU. @@ -127,9 +134,15 @@ struct aggr_cpu_id aggr_cpu_id__socket(struct perf_cpu cpu, void *data); */ struct aggr_cpu_id aggr_cpu_id__die(struct perf_cpu cpu, void *data); /** - * aggr_cpu_id__core - Create an aggr_cpu_id with the core, die and socket - * populated with the core, die and socket for cpu. The function signature is - * compatible with aggr_cpu_id_get_t. + * aggr_cpu_id__cluster - Create an aggr_cpu_id with cluster, die and socket + * populated with the cluster, die and socket for cpu. The function signature + * is compatible with aggr_cpu_id_get_t. + */ +struct aggr_cpu_id aggr_cpu_id__cluster(struct perf_cpu cpu, void *data); +/** + * aggr_cpu_id__core - Create an aggr_cpu_id with the core, cluster, die and + * socket populated with the core, die and socket for cpu. The function + * signature is compatible with aggr_cpu_id_get_t. */ struct aggr_cpu_id aggr_cpu_id__core(struct perf_cpu cpu, void *data); /** diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h index 7c527e65c186..2a2c37cc40b7 100644 --- a/tools/perf/util/env.h +++ b/tools/perf/util/env.h @@ -12,6 +12,7 @@ struct perf_cpu_map; struct cpu_topology_map { int socket_id; int die_id; + int cluster_id; int core_id; }; diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c index 8c61f8627ebc..4dfe7d9517a9 100644 --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c @@ -201,6 +201,9 @@ static void print_aggr_id_std(struct perf_stat_config *config, snprintf(buf, sizeof(buf), "S%d-D%d-L%d-ID%d", id.socket, id.die, id.cache_lvl, id.cache); break; + case AGGR_CLUSTER: + snprintf(buf, sizeof(buf), "S%d-D%d-CLS%d", id.socket, id.die, id.cluster); + break; case AGGR_DIE: snprintf(buf, sizeof(buf), "S%d-D%d", id.socket, id.die); break; @@ -251,6 +254,10 @@ static void print_aggr_id_csv(struct perf_stat_config *config, fprintf(config->output, "S%d-D%d-L%d-ID%d%s%d%s", id.socket, id.die, id.cache_lvl, id.cache, sep, aggr_nr, sep); break; + case AGGR_CLUSTER: + fprintf(config->output, "S%d-D%d-CLS%d%s%d%s", + id.socket, id.die, id.cluster, sep, aggr_nr, sep); + break; case AGGR_DIE: fprintf(output, "S%d-D%d%s%d%s", id.socket, id.die, sep, aggr_nr, sep); @@ -300,6 +307,10 @@ static void print_aggr_id_json(struct perf_stat_config *config, fprintf(output, "\"cache\" : \"S%d-D%d-L%d-ID%d\", \"aggregate-number\" : %d, ", id.socket, id.die, id.cache_lvl, id.cache, aggr_nr); break; + case AGGR_CLUSTER: + fprintf(output, "\"cluster\" : \"S%d-D%d-CLS%d\", \"aggregate-number\" : %d, ", + id.socket, id.die, id.cluster, aggr_nr); + break; case AGGR_DIE: fprintf(output, "\"die\" : \"S%d-D%d\", \"aggregate-number\" : %d, ", id.socket, id.die, aggr_nr); @@ -1248,6 +1259,7 @@ static void print_header_interval_std(struct perf_stat_config *config, case AGGR_NODE: case AGGR_SOCKET: case AGGR_DIE: + case AGGR_CLUSTER: case AGGR_CACHE: case AGGR_CORE: fprintf(output, "#%*s %-*s cpus", @@ -1550,6 +1562,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf switch (config->aggr_mode) { case AGGR_CORE: case AGGR_CACHE: + case AGGR_CLUSTER: case AGGR_DIE: case AGGR_SOCKET: case AGGR_NODE: diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h index 4357ba114822..d6e5c8787ba2 100644 --- a/tools/perf/util/stat.h +++ b/tools/perf/util/stat.h @@ -48,6 +48,7 @@ enum aggr_mode { AGGR_GLOBAL, AGGR_SOCKET, AGGR_DIE, + AGGR_CLUSTER, AGGR_CACHE, AGGR_CORE, AGGR_THREAD, -- cgit v1.2.3 From 659ad3492b913c9033d47cb406ac5754780875b6 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 9 Feb 2024 19:17:41 -0800 Subject: perf maps: Switch from rbtree to lazily sorted array for addresses Maps is a collection of maps primarily sorted by the starting address of the map. Prior to this change the maps were held in an rbtree requiring 4 pointers per node. Prior to reference count checking, the rbnode was embedded in the map so 3 pointers per node were necessary. This change switches the rbtree to an array lazily sorted by address, much as the array sorting nodes by name. 1 pointer is needed per node, but to avoid excessive resizing the backing array may be twice the number of used elements. Meaning the memory overhead is roughly half that of the rbtree. For a perf record with "--no-bpf-event -g -a" of true, the memory overhead of perf inject is reduce fom 3.3MB to 3MB, so 10% or 300KB is saved. Map inserts always happen at the end of the array. The code tracks whether the insertion violates the sorting property. O(log n) rb-tree complexity is switched to O(1). Remove slides the array, so O(log n) rb-tree complexity is degraded to O(n). A find may need to sort the array using qsort which is O(n*log n), but in general the maps should be sorted and so average performance should be O(log n) as with the rbtree. An rbtree node consumes a cache line, but with the array 4 nodes fit on a cache line. Iteration is simplified to scanning an array rather than pointer chasing. Overall it is expected the performance after the change should be comparable to before, but with half of the memory consumed. To avoid a list and repeated logic around splitting maps, maps__merge_in is rewritten in terms of maps__fixup_overlap_and_insert. maps_merge_in splits the given mapping inserting remaining gaps. maps__fixup_overlap_and_insert splits the existing mappings, then adds the incoming mapping. By adding the new mapping first, then re-inserting the existing mappings the splitting behavior matches. Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Cc: K Prateek Nayak Cc: James Clark Cc: Vincent Whitchurch Cc: Alexey Dobriyan Cc: Colin Ian King Cc: Changbin Du Cc: Masami Hiramatsu Cc: Song Liu Cc: Leo Yan Cc: Athira Rajeev Cc: Liam Howlett Cc: Artem Savkov Cc: bpf@vger.kernel.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240210031746.4057262-2-irogers@google.com --- tools/perf/tests/maps.c | 3 + tools/perf/util/map.c | 1 + tools/perf/util/maps.c | 1203 ++++++++++++++++++++++++++++------------------- tools/perf/util/maps.h | 54 ++- 4 files changed, 777 insertions(+), 484 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c index bb3fbfe5a73e..b15417a0d617 100644 --- a/tools/perf/tests/maps.c +++ b/tools/perf/tests/maps.c @@ -156,6 +156,9 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest TEST_ASSERT_VAL("merge check failed", !ret); maps__zput(maps); + map__zput(map_kcore1); + map__zput(map_kcore2); + map__zput(map_kcore3); return TEST_OK; } diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 7a785a47467e..14a5ea70d81e 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -168,6 +168,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len, if (dso == NULL) goto out_delete; + assert(!dso->kernel); map__init(result, start, start + len, pgoff, dso); if (anon || no_dso) { diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c index 0334fc18d9c6..13dec408b931 100644 --- a/tools/perf/util/maps.c +++ b/tools/perf/util/maps.c @@ -10,286 +10,496 @@ #include "ui/ui.h" #include "unwind.h" -struct map_rb_node { - struct rb_node rb_node; - struct map *map; -}; - -#define maps__for_each_entry(maps, map) \ - for (map = maps__first(maps); map; map = map_rb_node__next(map)) +static void check_invariants(const struct maps *maps __maybe_unused) +{ +#ifndef NDEBUG + assert(RC_CHK_ACCESS(maps)->nr_maps <= RC_CHK_ACCESS(maps)->nr_maps_allocated); + for (unsigned int i = 0; i < RC_CHK_ACCESS(maps)->nr_maps; i++) { + struct map *map = RC_CHK_ACCESS(maps)->maps_by_address[i]; + + /* Check map is well-formed. */ + assert(map__end(map) == 0 || map__start(map) <= map__end(map)); + /* Expect at least 1 reference count. */ + assert(refcount_read(map__refcnt(map)) > 0); + + if (map__dso(map) && map__dso(map)->kernel) + assert(RC_CHK_EQUAL(map__kmap(map)->kmaps, maps)); + + if (i > 0) { + struct map *prev = RC_CHK_ACCESS(maps)->maps_by_address[i - 1]; + + /* If addresses are sorted... */ + if (RC_CHK_ACCESS(maps)->maps_by_address_sorted) { + /* Maps should be in start address order. */ + assert(map__start(prev) <= map__start(map)); + /* + * If the ends of maps aren't broken (during + * construction) then they should be ordered + * too. + */ + if (!RC_CHK_ACCESS(maps)->ends_broken) { + assert(map__end(prev) <= map__end(map)); + assert(map__end(prev) <= map__start(map) || + map__start(prev) == map__start(map)); + } + } + } + } + if (RC_CHK_ACCESS(maps)->maps_by_name) { + for (unsigned int i = 0; i < RC_CHK_ACCESS(maps)->nr_maps; i++) { + struct map *map = RC_CHK_ACCESS(maps)->maps_by_name[i]; -#define maps__for_each_entry_safe(maps, map, next) \ - for (map = maps__first(maps), next = map_rb_node__next(map); map; \ - map = next, next = map_rb_node__next(map)) + /* + * Maps by name maps should be in maps_by_address, so + * the reference count should be higher. + */ + assert(refcount_read(map__refcnt(map)) > 1); + } + } +#endif +} -static struct rb_root *maps__entries(struct maps *maps) +static struct map **maps__maps_by_address(const struct maps *maps) { - return &RC_CHK_ACCESS(maps)->entries; + return RC_CHK_ACCESS(maps)->maps_by_address; } -static struct rw_semaphore *maps__lock(struct maps *maps) +static void maps__set_maps_by_address(struct maps *maps, struct map **new) { - return &RC_CHK_ACCESS(maps)->lock; + RC_CHK_ACCESS(maps)->maps_by_address = new; + } -static struct map **maps__maps_by_name(struct maps *maps) +static struct map ***maps__maps_by_name_addr(struct maps *maps) { - return RC_CHK_ACCESS(maps)->maps_by_name; + return &RC_CHK_ACCESS(maps)->maps_by_name; } -static struct map_rb_node *maps__first(struct maps *maps) +static void maps__set_nr_maps_allocated(struct maps *maps, unsigned int nr_maps_allocated) { - struct rb_node *first = rb_first(maps__entries(maps)); + RC_CHK_ACCESS(maps)->nr_maps_allocated = nr_maps_allocated; +} - if (first) - return rb_entry(first, struct map_rb_node, rb_node); - return NULL; +static void maps__set_nr_maps(struct maps *maps, unsigned int nr_maps) +{ + RC_CHK_ACCESS(maps)->nr_maps = nr_maps; } -static struct map_rb_node *map_rb_node__next(struct map_rb_node *node) +/* Not in the header, to aid reference counting. */ +static struct map **maps__maps_by_name(const struct maps *maps) { - struct rb_node *next; + return RC_CHK_ACCESS(maps)->maps_by_name; - if (!node) - return NULL; +} - next = rb_next(&node->rb_node); +static void maps__set_maps_by_name(struct maps *maps, struct map **new) +{ + RC_CHK_ACCESS(maps)->maps_by_name = new; - if (!next) - return NULL; +} - return rb_entry(next, struct map_rb_node, rb_node); +static bool maps__maps_by_address_sorted(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->maps_by_address_sorted; } -static struct map_rb_node *maps__find_node(struct maps *maps, struct map *map) +static void maps__set_maps_by_address_sorted(struct maps *maps, bool value) { - struct map_rb_node *rb_node; + RC_CHK_ACCESS(maps)->maps_by_address_sorted = value; +} - maps__for_each_entry(maps, rb_node) { - if (rb_node->RC_CHK_ACCESS(map) == RC_CHK_ACCESS(map)) - return rb_node; - } - return NULL; +static bool maps__maps_by_name_sorted(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->maps_by_name_sorted; } -static void maps__init(struct maps *maps, struct machine *machine) +static void maps__set_maps_by_name_sorted(struct maps *maps, bool value) { - refcount_set(maps__refcnt(maps), 1); - init_rwsem(maps__lock(maps)); - RC_CHK_ACCESS(maps)->entries = RB_ROOT; - RC_CHK_ACCESS(maps)->machine = machine; - RC_CHK_ACCESS(maps)->last_search_by_name = NULL; - RC_CHK_ACCESS(maps)->nr_maps = 0; - RC_CHK_ACCESS(maps)->maps_by_name = NULL; + RC_CHK_ACCESS(maps)->maps_by_name_sorted = value; } -static void __maps__free_maps_by_name(struct maps *maps) +static struct rw_semaphore *maps__lock(struct maps *maps) { /* - * Free everything to try to do it from the rbtree in the next search + * When the lock is acquired or released the maps invariants should + * hold. */ - for (unsigned int i = 0; i < maps__nr_maps(maps); i++) - map__put(maps__maps_by_name(maps)[i]); + check_invariants(maps); + return &RC_CHK_ACCESS(maps)->lock; +} - zfree(&RC_CHK_ACCESS(maps)->maps_by_name); +static void maps__init(struct maps *maps, struct machine *machine) +{ + init_rwsem(maps__lock(maps)); + RC_CHK_ACCESS(maps)->maps_by_address = NULL; + RC_CHK_ACCESS(maps)->maps_by_name = NULL; + RC_CHK_ACCESS(maps)->machine = machine; +#ifdef HAVE_LIBUNWIND_SUPPORT + RC_CHK_ACCESS(maps)->addr_space = NULL; + RC_CHK_ACCESS(maps)->unwind_libunwind_ops = NULL; +#endif + refcount_set(maps__refcnt(maps), 1); + RC_CHK_ACCESS(maps)->nr_maps = 0; RC_CHK_ACCESS(maps)->nr_maps_allocated = 0; + RC_CHK_ACCESS(maps)->last_search_by_name_idx = 0; + RC_CHK_ACCESS(maps)->maps_by_address_sorted = true; + RC_CHK_ACCESS(maps)->maps_by_name_sorted = false; } -static int __maps__insert(struct maps *maps, struct map *map) +static void maps__exit(struct maps *maps) { - struct rb_node **p = &maps__entries(maps)->rb_node; - struct rb_node *parent = NULL; - const u64 ip = map__start(map); - struct map_rb_node *m, *new_rb_node; + struct map **maps_by_address = maps__maps_by_address(maps); + struct map **maps_by_name = maps__maps_by_name(maps); - new_rb_node = malloc(sizeof(*new_rb_node)); - if (!new_rb_node) - return -ENOMEM; + for (unsigned int i = 0; i < maps__nr_maps(maps); i++) { + map__zput(maps_by_address[i]); + if (maps_by_name) + map__zput(maps_by_name[i]); + } + zfree(&maps_by_address); + zfree(&maps_by_name); + unwind__finish_access(maps); +} - RB_CLEAR_NODE(&new_rb_node->rb_node); - new_rb_node->map = map__get(map); +struct maps *maps__new(struct machine *machine) +{ + struct maps *result; + RC_STRUCT(maps) *maps = zalloc(sizeof(*maps)); - while (*p != NULL) { - parent = *p; - m = rb_entry(parent, struct map_rb_node, rb_node); - if (ip < map__start(m->map)) - p = &(*p)->rb_left; - else - p = &(*p)->rb_right; - } + if (ADD_RC_CHK(result, maps)) + maps__init(result, machine); - rb_link_node(&new_rb_node->rb_node, parent, p); - rb_insert_color(&new_rb_node->rb_node, maps__entries(maps)); - return 0; + return result; } -int maps__insert(struct maps *maps, struct map *map) +static void maps__delete(struct maps *maps) { - int err; - const struct dso *dso = map__dso(map); - - down_write(maps__lock(maps)); - err = __maps__insert(maps, map); - if (err) - goto out; + maps__exit(maps); + RC_CHK_FREE(maps); +} - ++RC_CHK_ACCESS(maps)->nr_maps; +struct maps *maps__get(struct maps *maps) +{ + struct maps *result; - if (dso && dso->kernel) { - struct kmap *kmap = map__kmap(map); + if (RC_CHK_GET(result, maps)) + refcount_inc(maps__refcnt(maps)); - if (kmap) - kmap->kmaps = maps; - else - pr_err("Internal error: kernel dso with non kernel map\n"); - } + return result; +} +void maps__put(struct maps *maps) +{ + if (maps && refcount_dec_and_test(maps__refcnt(maps))) + maps__delete(maps); + else + RC_CHK_PUT(maps); +} +static void __maps__free_maps_by_name(struct maps *maps) +{ /* - * If we already performed some search by name, then we need to add the just - * inserted map and resort. + * Free everything to try to do it from the rbtree in the next search */ - if (maps__maps_by_name(maps)) { - if (maps__nr_maps(maps) > RC_CHK_ACCESS(maps)->nr_maps_allocated) { - int nr_allocate = maps__nr_maps(maps) * 2; - struct map **maps_by_name = realloc(maps__maps_by_name(maps), - nr_allocate * sizeof(map)); + for (unsigned int i = 0; i < maps__nr_maps(maps); i++) + map__put(maps__maps_by_name(maps)[i]); - if (maps_by_name == NULL) { - __maps__free_maps_by_name(maps); - err = -ENOMEM; - goto out; - } + zfree(&RC_CHK_ACCESS(maps)->maps_by_name); +} - RC_CHK_ACCESS(maps)->maps_by_name = maps_by_name; - RC_CHK_ACCESS(maps)->nr_maps_allocated = nr_allocate; +static int map__start_cmp(const void *a, const void *b) +{ + const struct map *map_a = *(const struct map * const *)a; + const struct map *map_b = *(const struct map * const *)b; + u64 map_a_start = map__start(map_a); + u64 map_b_start = map__start(map_b); + + if (map_a_start == map_b_start) { + u64 map_a_end = map__end(map_a); + u64 map_b_end = map__end(map_b); + + if (map_a_end == map_b_end) { + /* Ensure maps with the same addresses have a fixed order. */ + if (RC_CHK_ACCESS(map_a) == RC_CHK_ACCESS(map_b)) + return 0; + return (intptr_t)RC_CHK_ACCESS(map_a) > (intptr_t)RC_CHK_ACCESS(map_b) + ? 1 : -1; } - maps__maps_by_name(maps)[maps__nr_maps(maps) - 1] = map__get(map); - __maps__sort_by_name(maps); + return map_a_end > map_b_end ? 1 : -1; } - out: - up_write(maps__lock(maps)); - return err; + return map_a_start > map_b_start ? 1 : -1; } -static void __maps__remove(struct maps *maps, struct map_rb_node *rb_node) +static void __maps__sort_by_address(struct maps *maps) { - rb_erase_init(&rb_node->rb_node, maps__entries(maps)); - map__put(rb_node->map); - free(rb_node); + if (maps__maps_by_address_sorted(maps)) + return; + + qsort(maps__maps_by_address(maps), + maps__nr_maps(maps), + sizeof(struct map *), + map__start_cmp); + maps__set_maps_by_address_sorted(maps, true); } -void maps__remove(struct maps *maps, struct map *map) +static void maps__sort_by_address(struct maps *maps) { - struct map_rb_node *rb_node; - down_write(maps__lock(maps)); - if (RC_CHK_ACCESS(maps)->last_search_by_name == map) - RC_CHK_ACCESS(maps)->last_search_by_name = NULL; - - rb_node = maps__find_node(maps, map); - assert(rb_node->RC_CHK_ACCESS(map) == RC_CHK_ACCESS(map)); - __maps__remove(maps, rb_node); - if (maps__maps_by_name(maps)) - __maps__free_maps_by_name(maps); - --RC_CHK_ACCESS(maps)->nr_maps; + __maps__sort_by_address(maps); up_write(maps__lock(maps)); } -static void __maps__purge(struct maps *maps) +static int map__strcmp(const void *a, const void *b) { - struct map_rb_node *pos, *next; - - if (maps__maps_by_name(maps)) - __maps__free_maps_by_name(maps); + const struct map *map_a = *(const struct map * const *)a; + const struct map *map_b = *(const struct map * const *)b; + const struct dso *dso_a = map__dso(map_a); + const struct dso *dso_b = map__dso(map_b); + int ret = strcmp(dso_a->short_name, dso_b->short_name); - maps__for_each_entry_safe(maps, pos, next) { - rb_erase_init(&pos->rb_node, maps__entries(maps)); - map__put(pos->map); - free(pos); + if (ret == 0 && RC_CHK_ACCESS(map_a) != RC_CHK_ACCESS(map_b)) { + /* Ensure distinct but name equal maps have an order. */ + return map__start_cmp(a, b); } + return ret; } -static void maps__exit(struct maps *maps) +static int maps__sort_by_name(struct maps *maps) { + int err = 0; down_write(maps__lock(maps)); - __maps__purge(maps); + if (!maps__maps_by_name_sorted(maps)) { + struct map **maps_by_name = maps__maps_by_name(maps); + + if (!maps_by_name) { + maps_by_name = malloc(RC_CHK_ACCESS(maps)->nr_maps_allocated * + sizeof(*maps_by_name)); + if (!maps_by_name) + err = -ENOMEM; + else { + struct map **maps_by_address = maps__maps_by_address(maps); + unsigned int n = maps__nr_maps(maps); + + maps__set_maps_by_name(maps, maps_by_name); + for (unsigned int i = 0; i < n; i++) + maps_by_name[i] = map__get(maps_by_address[i]); + } + } + if (!err) { + qsort(maps_by_name, + maps__nr_maps(maps), + sizeof(struct map *), + map__strcmp); + maps__set_maps_by_name_sorted(maps, true); + } + } up_write(maps__lock(maps)); + return err; } -bool maps__empty(struct maps *maps) +static unsigned int maps__by_address_index(const struct maps *maps, const struct map *map) { - return !maps__first(maps); + struct map **maps_by_address = maps__maps_by_address(maps); + + if (maps__maps_by_address_sorted(maps)) { + struct map **mapp = + bsearch(&map, maps__maps_by_address(maps), maps__nr_maps(maps), + sizeof(*mapp), map__start_cmp); + + if (mapp) + return mapp - maps_by_address; + } else { + for (unsigned int i = 0; i < maps__nr_maps(maps); i++) { + if (RC_CHK_ACCESS(maps_by_address[i]) == RC_CHK_ACCESS(map)) + return i; + } + } + pr_err("Map missing from maps"); + return -1; } -struct maps *maps__new(struct machine *machine) +static unsigned int maps__by_name_index(const struct maps *maps, const struct map *map) { - struct maps *result; - RC_STRUCT(maps) *maps = zalloc(sizeof(*maps)); + struct map **maps_by_name = maps__maps_by_name(maps); + + if (maps__maps_by_name_sorted(maps)) { + struct map **mapp = + bsearch(&map, maps_by_name, maps__nr_maps(maps), + sizeof(*mapp), map__strcmp); + + if (mapp) + return mapp - maps_by_name; + } else { + for (unsigned int i = 0; i < maps__nr_maps(maps); i++) { + if (RC_CHK_ACCESS(maps_by_name[i]) == RC_CHK_ACCESS(map)) + return i; + } + } + pr_err("Map missing from maps"); + return -1; +} - if (ADD_RC_CHK(result, maps)) - maps__init(result, machine); +static int __maps__insert(struct maps *maps, struct map *new) +{ + struct map **maps_by_address = maps__maps_by_address(maps); + struct map **maps_by_name = maps__maps_by_name(maps); + const struct dso *dso = map__dso(new); + unsigned int nr_maps = maps__nr_maps(maps); + unsigned int nr_allocate = RC_CHK_ACCESS(maps)->nr_maps_allocated; + + if (nr_maps + 1 > nr_allocate) { + nr_allocate = !nr_allocate ? 32 : nr_allocate * 2; + + maps_by_address = realloc(maps_by_address, nr_allocate * sizeof(new)); + if (!maps_by_address) + return -ENOMEM; + + maps__set_maps_by_address(maps, maps_by_address); + if (maps_by_name) { + maps_by_name = realloc(maps_by_name, nr_allocate * sizeof(new)); + if (!maps_by_name) { + /* + * If by name fails, just disable by name and it will + * recompute next time it is required. + */ + __maps__free_maps_by_name(maps); + } + maps__set_maps_by_name(maps, maps_by_name); + } + RC_CHK_ACCESS(maps)->nr_maps_allocated = nr_allocate; + } + /* Insert the value at the end. */ + maps_by_address[nr_maps] = map__get(new); + if (maps_by_name) + maps_by_name[nr_maps] = map__get(new); - return result; + nr_maps++; + RC_CHK_ACCESS(maps)->nr_maps = nr_maps; + + /* + * Recompute if things are sorted. If things are inserted in a sorted + * manner, for example by processing /proc/pid/maps, then no + * sorting/resorting will be necessary. + */ + if (nr_maps == 1) { + /* If there's just 1 entry then maps are sorted. */ + maps__set_maps_by_address_sorted(maps, true); + maps__set_maps_by_name_sorted(maps, maps_by_name != NULL); + } else { + /* Sorted if maps were already sorted and this map starts after the last one. */ + maps__set_maps_by_address_sorted(maps, + maps__maps_by_address_sorted(maps) && + map__end(maps_by_address[nr_maps - 2]) <= map__start(new)); + maps__set_maps_by_name_sorted(maps, false); + } + if (map__end(new) < map__start(new)) + RC_CHK_ACCESS(maps)->ends_broken = true; + if (dso && dso->kernel) { + struct kmap *kmap = map__kmap(new); + + if (kmap) + kmap->kmaps = maps; + else + pr_err("Internal error: kernel dso with non kernel map\n"); + } + return 0; } -static void maps__delete(struct maps *maps) +int maps__insert(struct maps *maps, struct map *map) { - maps__exit(maps); - unwind__finish_access(maps); - RC_CHK_FREE(maps); + int ret; + + down_write(maps__lock(maps)); + ret = __maps__insert(maps, map); + up_write(maps__lock(maps)); + return ret; } -struct maps *maps__get(struct maps *maps) +static void __maps__remove(struct maps *maps, struct map *map) { - struct maps *result; + struct map **maps_by_address = maps__maps_by_address(maps); + struct map **maps_by_name = maps__maps_by_name(maps); + unsigned int nr_maps = maps__nr_maps(maps); + unsigned int address_idx; + + /* Slide later mappings over the one to remove */ + address_idx = maps__by_address_index(maps, map); + map__put(maps_by_address[address_idx]); + memmove(&maps_by_address[address_idx], + &maps_by_address[address_idx + 1], + (nr_maps - address_idx - 1) * sizeof(*maps_by_address)); + + if (maps_by_name) { + unsigned int name_idx = maps__by_name_index(maps, map); + + map__put(maps_by_name[name_idx]); + memmove(&maps_by_name[name_idx], + &maps_by_name[name_idx + 1], + (nr_maps - name_idx - 1) * sizeof(*maps_by_name)); + } - if (RC_CHK_GET(result, maps)) - refcount_inc(maps__refcnt(maps)); + --RC_CHK_ACCESS(maps)->nr_maps; +} - return result; +void maps__remove(struct maps *maps, struct map *map) +{ + down_write(maps__lock(maps)); + __maps__remove(maps, map); + up_write(maps__lock(maps)); } -void maps__put(struct maps *maps) +bool maps__empty(struct maps *maps) { - if (maps && refcount_dec_and_test(maps__refcnt(maps))) - maps__delete(maps); - else - RC_CHK_PUT(maps); + return maps__nr_maps(maps) == 0; } int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data), void *data) { - struct map_rb_node *pos; + bool done = false; int ret = 0; - down_read(maps__lock(maps)); - maps__for_each_entry(maps, pos) { - ret = cb(pos->map, data); - if (ret) - break; + /* See locking/sorting note. */ + while (!done) { + down_read(maps__lock(maps)); + if (maps__maps_by_address_sorted(maps)) { + /* + * maps__for_each_map callbacks may buggily/unsafely + * insert into maps_by_address. Deliberately reload + * maps__nr_maps and maps_by_address on each iteration + * to avoid using memory freed by maps__insert growing + * the array - this may cause maps to be skipped or + * repeated. + */ + for (unsigned int i = 0; i < maps__nr_maps(maps); i++) { + struct map **maps_by_address = maps__maps_by_address(maps); + struct map *map = maps_by_address[i]; + + ret = cb(map, data); + if (ret) + break; + } + done = true; + } + up_read(maps__lock(maps)); + if (!done) + maps__sort_by_address(maps); } - up_read(maps__lock(maps)); return ret; } void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data), void *data) { - struct map_rb_node *pos, *next; - unsigned int start_nr_maps; + struct map **maps_by_address; down_write(maps__lock(maps)); - start_nr_maps = maps__nr_maps(maps); - maps__for_each_entry_safe(maps, pos, next) { - if (cb(pos->map, data)) { - __maps__remove(maps, pos); - --RC_CHK_ACCESS(maps)->nr_maps; - } + maps_by_address = maps__maps_by_address(maps); + for (unsigned int i = 0; i < maps__nr_maps(maps);) { + if (cb(maps_by_address[i], data)) + __maps__remove(maps, maps_by_address[i]); + else + i++; } - if (maps__maps_by_name(maps) && start_nr_maps != maps__nr_maps(maps)) - __maps__free_maps_by_name(maps); - up_write(maps__lock(maps)); } @@ -300,7 +510,7 @@ struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp) /* Ensure map is loaded before using map->map_ip */ if (map != NULL && map__load(map) >= 0) { if (mapp != NULL) - *mapp = map; + *mapp = map; // TODO: map_put on else path when find returns a get. return map__find_symbol(map, map__map_ip(map, addr)); } @@ -348,7 +558,7 @@ int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams) if (ams->addr < map__start(ams->ms.map) || ams->addr >= map__end(ams->ms.map)) { if (maps == NULL) return -1; - ams->ms.map = maps__find(maps, ams->addr); + ams->ms.map = maps__find(maps, ams->addr); // TODO: map_get if (ams->ms.map == NULL) return -1; } @@ -393,24 +603,28 @@ size_t maps__fprintf(struct maps *maps, FILE *fp) * Find first map where end > map->start. * Same as find_vma() in kernel. */ -static struct rb_node *first_ending_after(struct maps *maps, const struct map *map) +static unsigned int first_ending_after(struct maps *maps, const struct map *map) { - struct rb_root *root; - struct rb_node *next, *first; + struct map **maps_by_address = maps__maps_by_address(maps); + int low = 0, high = (int)maps__nr_maps(maps) - 1, first = high + 1; + + assert(maps__maps_by_address_sorted(maps)); + if (low <= high && map__end(maps_by_address[0]) > map__start(map)) + return 0; - root = maps__entries(maps); - next = root->rb_node; - first = NULL; - while (next) { - struct map_rb_node *pos = rb_entry(next, struct map_rb_node, rb_node); + while (low <= high) { + int mid = (low + high) / 2; + struct map *pos = maps_by_address[mid]; - if (map__end(pos->map) > map__start(map)) { - first = next; - if (map__start(pos->map) <= map__start(map)) + if (map__end(pos) > map__start(map)) { + first = mid; + if (map__start(pos) <= map__start(map)) { + /* Entry overlaps map. */ break; - next = next->rb_left; + } + high = mid - 1; } else - next = next->rb_right; + low = mid + 1; } return first; } @@ -419,171 +633,249 @@ static struct rb_node *first_ending_after(struct maps *maps, const struct map *m * Adds new to maps, if new overlaps existing entries then the existing maps are * adjusted or removed so that new fits without overlapping any entries. */ -int maps__fixup_overlap_and_insert(struct maps *maps, struct map *new) +static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new) { - - struct rb_node *next; + struct map **maps_by_address; int err = 0; FILE *fp = debug_file(); - down_write(maps__lock(maps)); +sort_again: + if (!maps__maps_by_address_sorted(maps)) + __maps__sort_by_address(maps); - next = first_ending_after(maps, new); - while (next && !err) { - struct map_rb_node *pos = rb_entry(next, struct map_rb_node, rb_node); - next = rb_next(&pos->rb_node); + maps_by_address = maps__maps_by_address(maps); + /* + * Iterate through entries where the end of the existing entry is + * greater-than the new map's start. + */ + for (unsigned int i = first_ending_after(maps, new); i < maps__nr_maps(maps); ) { + struct map *pos = maps_by_address[i]; + struct map *before = NULL, *after = NULL; /* * Stop if current map starts after map->end. * Maps are ordered by start: next will not overlap for sure. */ - if (map__start(pos->map) >= map__end(new)) + if (map__start(pos) >= map__end(new)) break; - if (verbose >= 2) { - - if (use_browser) { - pr_debug("overlapping maps in %s (disable tui for more info)\n", - map__dso(new)->name); - } else { - pr_debug("overlapping maps:\n"); - map__fprintf(new, fp); - map__fprintf(pos->map, fp); - } + if (use_browser) { + pr_debug("overlapping maps in %s (disable tui for more info)\n", + map__dso(new)->name); + } else if (verbose >= 2) { + pr_debug("overlapping maps:\n"); + map__fprintf(new, fp); + map__fprintf(pos, fp); } - rb_erase_init(&pos->rb_node, maps__entries(maps)); /* * Now check if we need to create new maps for areas not * overlapped by the new map: */ - if (map__start(new) > map__start(pos->map)) { - struct map *before = map__clone(pos->map); + if (map__start(new) > map__start(pos)) { + /* Map starts within existing map. Need to shorten the existing map. */ + before = map__clone(pos); if (before == NULL) { err = -ENOMEM; - goto put_map; + goto out_err; } - map__set_end(before, map__start(new)); - err = __maps__insert(maps, before); - if (err) { - map__put(before); - goto put_map; - } if (verbose >= 2 && !use_browser) map__fprintf(before, fp); - map__put(before); } - - if (map__end(new) < map__end(pos->map)) { - struct map *after = map__clone(pos->map); + if (map__end(new) < map__end(pos)) { + /* The new map isn't as long as the existing map. */ + after = map__clone(pos); if (after == NULL) { + map__zput(before); err = -ENOMEM; - goto put_map; + goto out_err; } map__set_start(after, map__end(new)); - map__add_pgoff(after, map__end(new) - map__start(pos->map)); - assert(map__map_ip(pos->map, map__end(new)) == - map__map_ip(after, map__end(new))); - err = __maps__insert(maps, after); - if (err) { - map__put(after); - goto put_map; - } + map__add_pgoff(after, map__end(new) - map__start(pos)); + assert(map__map_ip(pos, map__end(new)) == + map__map_ip(after, map__end(new))); + if (verbose >= 2 && !use_browser) map__fprintf(after, fp); - map__put(after); } -put_map: - map__put(pos->map); - free(pos); + /* + * If adding one entry, for `before` or `after`, we can replace + * the existing entry. If both `before` and `after` are + * necessary than an insert is needed. If the existing entry + * entirely overlaps the existing entry it can just be removed. + */ + if (before) { + map__put(maps_by_address[i]); + maps_by_address[i] = before; + /* Maps are still ordered, go to next one. */ + i++; + if (after) { + __maps__insert(maps, after); + map__put(after); + if (!maps__maps_by_address_sorted(maps)) { + /* + * Sorting broken so invariants don't + * hold, sort and go again. + */ + goto sort_again; + } + /* + * Maps are still ordered, skip after and go to + * next one (terminate loop). + */ + i++; + } + } else if (after) { + map__put(maps_by_address[i]); + maps_by_address[i] = after; + /* Maps are ordered, go to next one. */ + i++; + } else { + __maps__remove(maps, pos); + /* + * Maps are ordered but no need to increase `i` as the + * later maps were moved down. + */ + } + check_invariants(maps); } /* Add the map. */ - err = __maps__insert(maps, new); - up_write(maps__lock(maps)); + __maps__insert(maps, new); +out_err: return err; } -int maps__copy_from(struct maps *maps, struct maps *parent) +int maps__fixup_overlap_and_insert(struct maps *maps, struct map *new) { int err; - struct map_rb_node *rb_node; + down_write(maps__lock(maps)); + err = __maps__fixup_overlap_and_insert(maps, new); + up_write(maps__lock(maps)); + return err; +} + +int maps__copy_from(struct maps *dest, struct maps *parent) +{ + /* Note, if struct map were immutable then cloning could use ref counts. */ + struct map **parent_maps_by_address; + int err = 0; + unsigned int n; + + down_write(maps__lock(dest)); down_read(maps__lock(parent)); - maps__for_each_entry(parent, rb_node) { - struct map *new = map__clone(rb_node->map); + parent_maps_by_address = maps__maps_by_address(parent); + n = maps__nr_maps(parent); + if (maps__empty(dest)) { + /* No existing mappings so just copy from parent to avoid reallocs in insert. */ + unsigned int nr_maps_allocated = RC_CHK_ACCESS(parent)->nr_maps_allocated; + struct map **dest_maps_by_address = + malloc(nr_maps_allocated * sizeof(struct map *)); + struct map **dest_maps_by_name = NULL; - if (new == NULL) { + if (!dest_maps_by_address) err = -ENOMEM; - goto out_unlock; + else { + if (maps__maps_by_name(parent)) { + dest_maps_by_name = + malloc(nr_maps_allocated * sizeof(struct map *)); + } + + RC_CHK_ACCESS(dest)->maps_by_address = dest_maps_by_address; + RC_CHK_ACCESS(dest)->maps_by_name = dest_maps_by_name; + RC_CHK_ACCESS(dest)->nr_maps_allocated = nr_maps_allocated; } - err = unwind__prepare_access(maps, new, NULL); - if (err) - goto out_unlock; + for (unsigned int i = 0; !err && i < n; i++) { + struct map *pos = parent_maps_by_address[i]; + struct map *new = map__clone(pos); - err = maps__insert(maps, new); - if (err) - goto out_unlock; + if (!new) + err = -ENOMEM; + else { + err = unwind__prepare_access(dest, new, NULL); + if (!err) { + dest_maps_by_address[i] = new; + if (dest_maps_by_name) + dest_maps_by_name[i] = map__get(new); + RC_CHK_ACCESS(dest)->nr_maps = i + 1; + } + } + if (err) + map__put(new); + } + maps__set_maps_by_address_sorted(dest, maps__maps_by_address_sorted(parent)); + if (!err) { + RC_CHK_ACCESS(dest)->last_search_by_name_idx = + RC_CHK_ACCESS(parent)->last_search_by_name_idx; + maps__set_maps_by_name_sorted(dest, + dest_maps_by_name && + maps__maps_by_name_sorted(parent)); + } else { + RC_CHK_ACCESS(dest)->last_search_by_name_idx = 0; + maps__set_maps_by_name_sorted(dest, false); + } + } else { + /* Unexpected copying to a maps containing entries. */ + for (unsigned int i = 0; !err && i < n; i++) { + struct map *pos = parent_maps_by_address[i]; + struct map *new = map__clone(pos); - map__put(new); + if (!new) + err = -ENOMEM; + else { + err = unwind__prepare_access(dest, new, NULL); + if (!err) + err = __maps__insert(dest, new); + } + map__put(new); + } } - - err = 0; -out_unlock: up_read(maps__lock(parent)); + up_write(maps__lock(dest)); return err; } -struct map *maps__find(struct maps *maps, u64 ip) +static int map__addr_cmp(const void *key, const void *entry) { - struct rb_node *p; - struct map_rb_node *m; - - - down_read(maps__lock(maps)); - - p = maps__entries(maps)->rb_node; - while (p != NULL) { - m = rb_entry(p, struct map_rb_node, rb_node); - if (ip < map__start(m->map)) - p = p->rb_left; - else if (ip >= map__end(m->map)) - p = p->rb_right; - else - goto out; - } + const u64 ip = *(const u64 *)key; + const struct map *map = *(const struct map * const *)entry; - m = NULL; -out: - up_read(maps__lock(maps)); - return m ? m->map : NULL; + if (ip < map__start(map)) + return -1; + if (ip >= map__end(map)) + return 1; + return 0; } -static int map__strcmp(const void *a, const void *b) +struct map *maps__find(struct maps *maps, u64 ip) { - const struct map *map_a = *(const struct map **)a; - const struct map *map_b = *(const struct map **)b; - const struct dso *dso_a = map__dso(map_a); - const struct dso *dso_b = map__dso(map_b); - int ret = strcmp(dso_a->short_name, dso_b->short_name); - - if (ret == 0 && map_a != map_b) { - /* - * Ensure distinct but name equal maps have an order in part to - * aid reference counting. - */ - ret = (int)map__start(map_a) - (int)map__start(map_b); - if (ret == 0) - ret = (int)((intptr_t)map_a - (intptr_t)map_b); + struct map *result = NULL; + bool done = false; + + /* See locking/sorting note. */ + while (!done) { + down_read(maps__lock(maps)); + if (maps__maps_by_address_sorted(maps)) { + struct map **mapp = + bsearch(&ip, maps__maps_by_address(maps), maps__nr_maps(maps), + sizeof(*mapp), map__addr_cmp); + + if (mapp) + result = *mapp; // map__get(*mapp); + done = true; + } + up_read(maps__lock(maps)); + if (!done) + maps__sort_by_address(maps); } - - return ret; + return result; } static int map__strcmp_name(const void *name, const void *b) @@ -593,126 +885,113 @@ static int map__strcmp_name(const void *name, const void *b) return strcmp(name, dso->short_name); } -void __maps__sort_by_name(struct maps *maps) -{ - qsort(maps__maps_by_name(maps), maps__nr_maps(maps), sizeof(struct map *), map__strcmp); -} - -static int map__groups__sort_by_name_from_rbtree(struct maps *maps) -{ - struct map_rb_node *rb_node; - struct map **maps_by_name = realloc(maps__maps_by_name(maps), - maps__nr_maps(maps) * sizeof(struct map *)); - int i = 0; - - if (maps_by_name == NULL) - return -1; - - up_read(maps__lock(maps)); - down_write(maps__lock(maps)); - - RC_CHK_ACCESS(maps)->maps_by_name = maps_by_name; - RC_CHK_ACCESS(maps)->nr_maps_allocated = maps__nr_maps(maps); - - maps__for_each_entry(maps, rb_node) - maps_by_name[i++] = map__get(rb_node->map); - - __maps__sort_by_name(maps); - - up_write(maps__lock(maps)); - down_read(maps__lock(maps)); - - return 0; -} - -static struct map *__maps__find_by_name(struct maps *maps, const char *name) +struct map *maps__find_by_name(struct maps *maps, const char *name) { - struct map **mapp; + struct map *result = NULL; + bool done = false; - if (maps__maps_by_name(maps) == NULL && - map__groups__sort_by_name_from_rbtree(maps)) - return NULL; + /* See locking/sorting note. */ + while (!done) { + unsigned int i; - mapp = bsearch(name, maps__maps_by_name(maps), maps__nr_maps(maps), - sizeof(*mapp), map__strcmp_name); - if (mapp) - return *mapp; - return NULL; -} + down_read(maps__lock(maps)); -struct map *maps__find_by_name(struct maps *maps, const char *name) -{ - struct map_rb_node *rb_node; - struct map *map; - - down_read(maps__lock(maps)); + /* First check last found entry. */ + i = RC_CHK_ACCESS(maps)->last_search_by_name_idx; + if (i < maps__nr_maps(maps) && maps__maps_by_name(maps)) { + struct dso *dso = map__dso(maps__maps_by_name(maps)[i]); + if (dso && strcmp(dso->short_name, name) == 0) { + result = maps__maps_by_name(maps)[i]; // TODO: map__get + done = true; + } + } - if (RC_CHK_ACCESS(maps)->last_search_by_name) { - const struct dso *dso = map__dso(RC_CHK_ACCESS(maps)->last_search_by_name); + /* Second search sorted array. */ + if (!done && maps__maps_by_name_sorted(maps)) { + struct map **mapp = + bsearch(name, maps__maps_by_name(maps), maps__nr_maps(maps), + sizeof(*mapp), map__strcmp_name); - if (strcmp(dso->short_name, name) == 0) { - map = RC_CHK_ACCESS(maps)->last_search_by_name; - goto out_unlock; + if (mapp) { + result = *mapp; // TODO: map__get + i = mapp - maps__maps_by_name(maps); + RC_CHK_ACCESS(maps)->last_search_by_name_idx = i; + } + done = true; } - } - /* - * If we have maps->maps_by_name, then the name isn't in the rbtree, - * as maps->maps_by_name mirrors the rbtree when lookups by name are - * made. - */ - map = __maps__find_by_name(maps, name); - if (map || maps__maps_by_name(maps) != NULL) - goto out_unlock; - - /* Fallback to traversing the rbtree... */ - maps__for_each_entry(maps, rb_node) { - struct dso *dso; - - map = rb_node->map; - dso = map__dso(map); - if (strcmp(dso->short_name, name) == 0) { - RC_CHK_ACCESS(maps)->last_search_by_name = map; - goto out_unlock; + up_read(maps__lock(maps)); + if (!done) { + /* Sort and retry binary search. */ + if (maps__sort_by_name(maps)) { + /* + * Memory allocation failed do linear search + * through address sorted maps. + */ + struct map **maps_by_address; + unsigned int n; + + down_read(maps__lock(maps)); + maps_by_address = maps__maps_by_address(maps); + n = maps__nr_maps(maps); + for (i = 0; i < n; i++) { + struct map *pos = maps_by_address[i]; + struct dso *dso = map__dso(pos); + + if (dso && strcmp(dso->short_name, name) == 0) { + result = pos; // TODO: map__get + break; + } + } + up_read(maps__lock(maps)); + done = true; + } } } - map = NULL; - -out_unlock: - up_read(maps__lock(maps)); - return map; + return result; } struct map *maps__find_next_entry(struct maps *maps, struct map *map) { - struct map_rb_node *rb_node = maps__find_node(maps, map); - struct map_rb_node *next = map_rb_node__next(rb_node); + unsigned int i; + struct map *result = NULL; - if (next) - return next->map; + down_read(maps__lock(maps)); + i = maps__by_address_index(maps, map); + if (i < maps__nr_maps(maps)) + result = maps__maps_by_address(maps)[i]; // TODO: map__get - return NULL; + up_read(maps__lock(maps)); + return result; } void maps__fixup_end(struct maps *maps) { - struct map_rb_node *prev = NULL, *curr; + struct map **maps_by_address; + unsigned int n; down_write(maps__lock(maps)); + if (!maps__maps_by_address_sorted(maps)) + __maps__sort_by_address(maps); - maps__for_each_entry(maps, curr) { - if (prev && (!map__end(prev->map) || map__end(prev->map) > map__start(curr->map))) - map__set_end(prev->map, map__start(curr->map)); + maps_by_address = maps__maps_by_address(maps); + n = maps__nr_maps(maps); + for (unsigned int i = 1; i < n; i++) { + struct map *prev = maps_by_address[i - 1]; + struct map *curr = maps_by_address[i]; - prev = curr; + if (!map__end(prev) || map__end(prev) > map__start(curr)) + map__set_end(prev, map__start(curr)); } /* * We still haven't the actual symbols, so guess the * last map final address. */ - if (curr && !map__end(curr->map)) - map__set_end(curr->map, ~0ULL); + if (n > 0 && !map__end(maps_by_address[n - 1])) + map__set_end(maps_by_address[n - 1], ~0ULL); + + RC_CHK_ACCESS(maps)->ends_broken = false; up_write(maps__lock(maps)); } @@ -723,117 +1002,93 @@ void maps__fixup_end(struct maps *maps) */ int maps__merge_in(struct maps *kmaps, struct map *new_map) { - struct map_rb_node *rb_node; - struct rb_node *first; - bool overlaps; - LIST_HEAD(merged); - int err = 0; - - down_read(maps__lock(kmaps)); - first = first_ending_after(kmaps, new_map); - rb_node = first ? rb_entry(first, struct map_rb_node, rb_node) : NULL; - overlaps = rb_node && map__start(rb_node->map) < map__end(new_map); - up_read(maps__lock(kmaps)); + unsigned int first_after_, kmaps__nr_maps; + struct map **kmaps_maps_by_address; + struct map **merged_maps_by_address; + unsigned int merged_nr_maps_allocated; + + /* First try under a read lock. */ + while (true) { + down_read(maps__lock(kmaps)); + if (maps__maps_by_address_sorted(kmaps)) + break; - if (!overlaps) - return maps__insert(kmaps, new_map); + up_read(maps__lock(kmaps)); - maps__for_each_entry(kmaps, rb_node) { - struct map *old_map = rb_node->map; + /* First after binary search requires sorted maps. Sort and try again. */ + maps__sort_by_address(kmaps); + } + first_after_ = first_ending_after(kmaps, new_map); + kmaps_maps_by_address = maps__maps_by_address(kmaps); - /* no overload with this one */ - if (map__end(new_map) < map__start(old_map) || - map__start(new_map) >= map__end(old_map)) - continue; + if (first_after_ >= maps__nr_maps(kmaps) || + map__start(kmaps_maps_by_address[first_after_]) >= map__end(new_map)) { + /* No overlap so regular insert suffices. */ + up_read(maps__lock(kmaps)); + return maps__insert(kmaps, new_map); + } + up_read(maps__lock(kmaps)); - if (map__start(new_map) < map__start(old_map)) { - /* - * |new...... - * |old.... - */ - if (map__end(new_map) < map__end(old_map)) { - /* - * |new......| -> |new..| - * |old....| -> |old....| - */ - map__set_end(new_map, map__start(old_map)); - } else { - /* - * |new.............| -> |new..| |new..| - * |old....| -> |old....| - */ - struct map_list_node *m = map_list_node__new(); + /* Plain insert with a read-lock failed, try again now with the write lock. */ + down_write(maps__lock(kmaps)); + if (!maps__maps_by_address_sorted(kmaps)) + __maps__sort_by_address(kmaps); + + first_after_ = first_ending_after(kmaps, new_map); + kmaps_maps_by_address = maps__maps_by_address(kmaps); + kmaps__nr_maps = maps__nr_maps(kmaps); + + if (first_after_ >= kmaps__nr_maps || + map__start(kmaps_maps_by_address[first_after_]) >= map__end(new_map)) { + /* No overlap so regular insert suffices. */ + int ret = __maps__insert(kmaps, new_map); + up_write(maps__lock(kmaps)); + return ret; + } + /* Array to merge into, possibly 1 more for the sake of new_map. */ + merged_nr_maps_allocated = RC_CHK_ACCESS(kmaps)->nr_maps_allocated; + if (kmaps__nr_maps + 1 == merged_nr_maps_allocated) + merged_nr_maps_allocated++; + + merged_maps_by_address = malloc(merged_nr_maps_allocated * sizeof(*merged_maps_by_address)); + if (!merged_maps_by_address) { + up_write(maps__lock(kmaps)); + return -ENOMEM; + } + maps__set_maps_by_address(kmaps, merged_maps_by_address); + maps__set_maps_by_address_sorted(kmaps, true); + zfree(maps__maps_by_name_addr(kmaps)); + maps__set_maps_by_name_sorted(kmaps, true); + maps__set_nr_maps_allocated(kmaps, merged_nr_maps_allocated); - if (!m) { - err = -ENOMEM; - goto out; - } + /* Copy entries before the new_map that can't overlap. */ + for (unsigned int i = 0; i < first_after_; i++) + merged_maps_by_address[i] = map__get(kmaps_maps_by_address[i]); - m->map = map__clone(new_map); - if (!m->map) { - free(m); - err = -ENOMEM; - goto out; - } + maps__set_nr_maps(kmaps, first_after_); - map__set_end(m->map, map__start(old_map)); - list_add_tail(&m->node, &merged); - map__add_pgoff(new_map, map__end(old_map) - map__start(new_map)); - map__set_start(new_map, map__end(old_map)); - } - } else { - /* - * |new...... - * |old.... - */ - if (map__end(new_map) < map__end(old_map)) { - /* - * |new..| -> x - * |old.........| -> |old.........| - */ - map__put(new_map); - new_map = NULL; - break; - } else { - /* - * |new......| -> |new...| - * |old....| -> |old....| - */ - map__add_pgoff(new_map, map__end(old_map) - map__start(new_map)); - map__set_start(new_map, map__end(old_map)); - } - } - } + /* Add the new map, it will be split when the later overlapping mappings are added. */ + __maps__insert(kmaps, new_map); -out: - while (!list_empty(&merged)) { - struct map_list_node *old_node; + /* Insert mappings after new_map, splitting new_map in the process. */ + for (unsigned int i = first_after_; i < kmaps__nr_maps; i++) + __maps__fixup_overlap_and_insert(kmaps, kmaps_maps_by_address[i]); - old_node = list_entry(merged.next, struct map_list_node, node); - list_del_init(&old_node->node); - if (!err) - err = maps__insert(kmaps, old_node->map); - map__put(old_node->map); - free(old_node); - } + /* Copy the maps from merged into kmaps. */ + for (unsigned int i = 0; i < kmaps__nr_maps; i++) + map__zput(kmaps_maps_by_address[i]); - if (new_map) { - if (!err) - err = maps__insert(kmaps, new_map); - map__put(new_map); - } - return err; + free(kmaps_maps_by_address); + up_write(maps__lock(kmaps)); + return 0; } void maps__load_first(struct maps *maps) { - struct map_rb_node *first; - down_read(maps__lock(maps)); - first = maps__first(maps); - if (first) - map__load(first->map); + if (maps__nr_maps(maps) > 0) + map__load(maps__maps_by_address(maps)[0]); up_read(maps__lock(maps)); } diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h index d836d04c9402..df9dd5a0e3c0 100644 --- a/tools/perf/util/maps.h +++ b/tools/perf/util/maps.h @@ -25,21 +25,56 @@ static inline struct map_list_node *map_list_node__new(void) return malloc(sizeof(struct map_list_node)); } -struct map *maps__find(struct maps *maps, u64 addr); +/* + * Locking/sorting note: + * + * Sorting is done with the write lock, iteration and binary searching happens + * under the read lock requiring being sorted. There is a race between sorting + * releasing the write lock and acquiring the read lock for iteration/searching + * where another thread could insert and break the sorting of the maps. In + * practice inserting maps should be rare meaning that the race shouldn't lead + * to live lock. Removal of maps doesn't break being sorted. + */ DECLARE_RC_STRUCT(maps) { - struct rb_root entries; struct rw_semaphore lock; - struct machine *machine; - struct map *last_search_by_name; + /** + * @maps_by_address: array of maps sorted by their starting address if + * maps_by_address_sorted is true. + */ + struct map **maps_by_address; + /** + * @maps_by_name: optional array of maps sorted by their dso name if + * maps_by_name_sorted is true. + */ struct map **maps_by_name; - refcount_t refcnt; - unsigned int nr_maps; - unsigned int nr_maps_allocated; + struct machine *machine; #ifdef HAVE_LIBUNWIND_SUPPORT - void *addr_space; + void *addr_space; const struct unwind_libunwind_ops *unwind_libunwind_ops; #endif + refcount_t refcnt; + /** + * @nr_maps: number of maps_by_address, and possibly maps_by_name, + * entries that contain maps. + */ + unsigned int nr_maps; + /** + * @nr_maps_allocated: number of entries in maps_by_address and possibly + * maps_by_name. + */ + unsigned int nr_maps_allocated; + /** + * @last_search_by_name_idx: cache of last found by name entry's index + * as frequent searches for the same dso name are common. + */ + unsigned int last_search_by_name_idx; + /** @maps_by_address_sorted: is maps_by_address sorted. */ + bool maps_by_address_sorted; + /** @maps_by_name_sorted: is maps_by_name sorted. */ + bool maps_by_name_sorted; + /** @ends_broken: does the map contain a map where end values are unset/unsorted? */ + bool ends_broken; }; #define KMAP_NAME_LEN 256 @@ -102,6 +137,7 @@ size_t maps__fprintf(struct maps *maps, FILE *fp); int maps__insert(struct maps *maps, struct map *map); void maps__remove(struct maps *maps, struct map *map); +struct map *maps__find(struct maps *maps, u64 addr); struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp); struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name, struct map **mapp); @@ -117,8 +153,6 @@ struct map *maps__find_next_entry(struct maps *maps, struct map *map); int maps__merge_in(struct maps *kmaps, struct map *new_map); -void __maps__sort_by_name(struct maps *maps); - void maps__fixup_end(struct maps *maps); void maps__load_first(struct maps *maps); -- cgit v1.2.3 From 42fd623b58dbcc48310705bbf3e3d4d7c1deec29 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 9 Feb 2024 19:17:42 -0800 Subject: perf maps: Get map before returning in maps__find Finding a map is done under a lock, returning the map without a reference count means it can be removed without notice and causing uses after free. Grab a reference count to the map within the lock region and return this. Fix up locations that need a map__put following this. Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Cc: K Prateek Nayak Cc: James Clark Cc: Vincent Whitchurch Cc: Alexey Dobriyan Cc: Colin Ian King Cc: Changbin Du Cc: Masami Hiramatsu Cc: Song Liu Cc: Leo Yan Cc: Athira Rajeev Cc: Liam Howlett Cc: Artem Savkov Cc: bpf@vger.kernel.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240210031746.4057262-3-irogers@google.com --- tools/perf/arch/x86/tests/dwarf-unwind.c | 1 + tools/perf/tests/vmlinux-kallsyms.c | 5 ++--- tools/perf/util/bpf-event.c | 1 + tools/perf/util/event.c | 4 ++-- tools/perf/util/machine.c | 22 ++++++++-------------- tools/perf/util/maps.c | 17 ++++++++++------- tools/perf/util/symbol.c | 3 ++- 7 files changed, 26 insertions(+), 27 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c index 5bfec3345d59..c05c0a85dad4 100644 --- a/tools/perf/arch/x86/tests/dwarf-unwind.c +++ b/tools/perf/arch/x86/tests/dwarf-unwind.c @@ -34,6 +34,7 @@ static int sample_ustack(struct perf_sample *sample, } stack_size = map__end(map) - sp; + map__put(map); stack_size = stack_size > STACK_SIZE ? STACK_SIZE : stack_size; memcpy(buf, (void *) sp, stack_size); diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index 822f893e67d5..e808e6fc8f76 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -151,10 +151,8 @@ static int test__vmlinux_matches_kallsyms_cb2(struct map *map, void *data) u64 mem_end = map__unmap_ip(args->vmlinux_map, map__end(map)); pair = maps__find(args->kallsyms.kmaps, mem_start); - if (pair == NULL || map__priv(pair)) - return 0; - if (map__start(pair) == mem_start) { + if (pair != NULL && !map__priv(pair) && map__start(pair) == mem_start) { struct dso *dso = map__dso(map); if (!args->header_printed) { @@ -170,6 +168,7 @@ static int test__vmlinux_matches_kallsyms_cb2(struct map *map, void *data) pr_info(" %s\n", dso->name); map__set_priv(pair, 1); } + map__put(pair); return 0; } diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c index 3573e0b7ef3e..83709146a48a 100644 --- a/tools/perf/util/bpf-event.c +++ b/tools/perf/util/bpf-event.c @@ -63,6 +63,7 @@ static int machine__process_bpf_event_load(struct machine *machine, dso->bpf_prog.id = id; dso->bpf_prog.sub_id = i; dso->bpf_prog.env = env; + map__put(map); } } return 0; diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 68f45e9e63b6..198903157f9e 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -511,7 +511,7 @@ size_t perf_event__fprintf_text_poke(union perf_event *event, struct machine *ma struct addr_location al; addr_location__init(&al); - al.map = map__get(maps__find(machine__kernel_maps(machine), tp->addr)); + al.map = maps__find(machine__kernel_maps(machine), tp->addr); if (al.map && map__load(al.map) >= 0) { al.addr = map__map_ip(al.map, tp->addr); al.sym = map__find_symbol(al.map, al.addr); @@ -641,7 +641,7 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr, return NULL; } al->maps = maps__get(maps); - al->map = map__get(maps__find(maps, al->addr)); + al->map = maps__find(maps, al->addr); if (al->map != NULL) { /* * Kernel maps might be changed when loading symbols so loading diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index b397a769006f..e8eb9f0b073f 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -896,7 +896,6 @@ static int machine__process_ksymbol_register(struct machine *machine, struct symbol *sym; struct dso *dso; struct map *map = maps__find(machine__kernel_maps(machine), event->ksymbol.addr); - bool put_map = false; int err = 0; if (!map) { @@ -913,12 +912,6 @@ static int machine__process_ksymbol_register(struct machine *machine, err = -ENOMEM; goto out; } - /* - * The inserted map has a get on it, we need to put to release - * the reference count here, but do it after all accesses are - * done. - */ - put_map = true; if (event->ksymbol.ksym_type == PERF_RECORD_KSYMBOL_TYPE_OOL) { dso->binary_type = DSO_BINARY_TYPE__OOL; dso->data.file_size = event->ksymbol.len; @@ -952,8 +945,7 @@ static int machine__process_ksymbol_register(struct machine *machine, } dso__insert_symbol(dso, sym); out: - if (put_map) - map__put(map); + map__put(map); return err; } @@ -977,7 +969,7 @@ static int machine__process_ksymbol_unregister(struct machine *machine, if (sym) dso__delete_symbol(dso, sym); } - + map__put(map); return 0; } @@ -1005,11 +997,11 @@ int machine__process_text_poke(struct machine *machine, union perf_event *event, perf_event__fprintf_text_poke(event, machine, stdout); if (!event->text_poke.new_len) - return 0; + goto out; if (cpumode != PERF_RECORD_MISC_KERNEL) { pr_debug("%s: unsupported cpumode - ignoring\n", __func__); - return 0; + goto out; } if (dso) { @@ -1032,7 +1024,8 @@ int machine__process_text_poke(struct machine *machine, union perf_event *event, pr_debug("Failed to find kernel text poke address map for %#" PRI_lx64 "\n", event->text_poke.addr); } - +out: + map__put(map); return 0; } @@ -1300,9 +1293,10 @@ static int machine__map_x86_64_entry_trampolines_cb(struct map *map, void *data) return 0; dest_map = maps__find(args->kmaps, map__pgoff(map)); - if (dest_map != map) + if (RC_CHK_ACCESS(dest_map) != RC_CHK_ACCESS(map)) map__set_pgoff(map, map__map_ip(dest_map, map__pgoff(map))); + map__put(dest_map); args->found = true; return 0; } diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c index 13dec408b931..2547c9074b3a 100644 --- a/tools/perf/util/maps.c +++ b/tools/perf/util/maps.c @@ -506,15 +506,18 @@ void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp) { struct map *map = maps__find(maps, addr); + struct symbol *result = NULL; /* Ensure map is loaded before using map->map_ip */ if (map != NULL && map__load(map) >= 0) { - if (mapp != NULL) - *mapp = map; // TODO: map_put on else path when find returns a get. - return map__find_symbol(map, map__map_ip(map, addr)); - } + if (mapp) + *mapp = map; - return NULL; + result = map__find_symbol(map, map__map_ip(map, addr)); + if (!mapp) + map__put(map); + } + return result; } struct maps__find_symbol_by_name_args { @@ -558,7 +561,7 @@ int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams) if (ams->addr < map__start(ams->ms.map) || ams->addr >= map__end(ams->ms.map)) { if (maps == NULL) return -1; - ams->ms.map = maps__find(maps, ams->addr); // TODO: map_get + ams->ms.map = maps__find(maps, ams->addr); if (ams->ms.map == NULL) return -1; } @@ -868,7 +871,7 @@ struct map *maps__find(struct maps *maps, u64 ip) sizeof(*mapp), map__addr_cmp); if (mapp) - result = *mapp; // map__get(*mapp); + result = map__get(*mapp); done = true; } up_read(maps__lock(maps)); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index be212ba157dc..1710b89e207c 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -757,7 +757,6 @@ static int dso__load_all_kallsyms(struct dso *dso, const char *filename) static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso) { - struct map *curr_map; struct symbol *pos; int count = 0; struct rb_root_cached old_root = dso->symbols; @@ -770,6 +769,7 @@ static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso) *root = RB_ROOT_CACHED; while (next) { + struct map *curr_map; struct dso *curr_map_dso; char *module; @@ -796,6 +796,7 @@ static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso) pos->end -= map__start(curr_map) - map__pgoff(curr_map); symbols__insert(&curr_map_dso->symbols, pos); ++count; + map__put(curr_map); } /* Symbols have been adjusted */ -- cgit v1.2.3 From 107ef66cb054f8d54e336236a31631a8cc167c1f Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 9 Feb 2024 19:17:43 -0800 Subject: perf maps: Get map before returning in maps__find_by_name Finding a map is done under a lock, returning the map without a reference count means it can be removed without notice and causing uses after free. Grab a reference count to the map within the lock region and return this. Fix up locations that need a map__put following this. Also fix some reference counted pointer comparisons. Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Cc: K Prateek Nayak Cc: James Clark Cc: Vincent Whitchurch Cc: Alexey Dobriyan Cc: Colin Ian King Cc: Changbin Du Cc: Masami Hiramatsu Cc: Song Liu Cc: Leo Yan Cc: Athira Rajeev Cc: Liam Howlett Cc: Artem Savkov Cc: bpf@vger.kernel.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240210031746.4057262-4-irogers@google.com --- tools/perf/tests/vmlinux-kallsyms.c | 5 +++-- tools/perf/util/machine.c | 6 ++++-- tools/perf/util/maps.c | 6 +++--- tools/perf/util/probe-event.c | 1 + tools/perf/util/symbol-elf.c | 4 +++- tools/perf/util/symbol.c | 18 +++++++++++------- 6 files changed, 25 insertions(+), 15 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index e808e6fc8f76..fecbf851bb2e 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -131,9 +131,10 @@ static int test__vmlinux_matches_kallsyms_cb1(struct map *map, void *data) struct map *pair = maps__find_by_name(args->kallsyms.kmaps, (dso->kernel ? dso->short_name : dso->name)); - if (pair) + if (pair) { map__set_priv(pair, 1); - else { + map__put(pair); + } else { if (!args->header_printed) { pr_info("WARN: Maps only in vmlinux:\n"); args->header_printed = true; diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index e8eb9f0b073f..7031f6fddcae 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -1537,8 +1537,10 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo return 0; long_name = strdup(path); - if (long_name == NULL) + if (long_name == NULL) { + map__put(map); return -ENOMEM; + } dso = map__dso(map); dso__set_long_name(dso, long_name, true); @@ -1552,7 +1554,7 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo dso->symtab_type++; dso->comp = m->comp; } - + map__put(map); return 0; } diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c index 2547c9074b3a..ea8fa684e8c6 100644 --- a/tools/perf/util/maps.c +++ b/tools/perf/util/maps.c @@ -905,7 +905,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name) struct dso *dso = map__dso(maps__maps_by_name(maps)[i]); if (dso && strcmp(dso->short_name, name) == 0) { - result = maps__maps_by_name(maps)[i]; // TODO: map__get + result = map__get(maps__maps_by_name(maps)[i]); done = true; } } @@ -917,7 +917,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name) sizeof(*mapp), map__strcmp_name); if (mapp) { - result = *mapp; // TODO: map__get + result = map__get(*mapp); i = mapp - maps__maps_by_name(maps); RC_CHK_ACCESS(maps)->last_search_by_name_idx = i; } @@ -942,7 +942,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name) struct dso *dso = map__dso(pos); if (dso && strcmp(dso->short_name, name) == 0) { - result = pos; // TODO: map__get + result = map__get(pos); break; } } diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index a1a796043691..be71abe8b9b0 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -358,6 +358,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso) map = maps__find_by_name(machine__kernel_maps(host_machine), module_name); if (map) { dso = map__dso(map); + map__put(map); goto found; } pr_debug("Failed to find module %s.\n", module); diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 95773c32796d..0b91f813c4fa 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1535,8 +1535,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, dso__set_loaded(curr_dso); *curr_mapp = curr_map; *curr_dsop = curr_dso; - } else + } else { *curr_dsop = map__dso(curr_map); + map__put(curr_map); + } return 0; } diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 1710b89e207c..0785a54e832e 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -814,7 +814,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, struct map *initial_map) { struct machine *machine; - struct map *curr_map = initial_map; + struct map *curr_map = map__get(initial_map); struct symbol *pos; int count = 0, moved = 0; struct rb_root_cached *root = &dso->symbols; @@ -858,13 +858,14 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, dso__set_loaded(curr_map_dso); } + map__zput(curr_map); curr_map = maps__find_by_name(kmaps, module); if (curr_map == NULL) { pr_debug("%s/proc/{kallsyms,modules} " "inconsistency while looking " "for \"%s\" module!\n", machine->root_dir, module); - curr_map = initial_map; + curr_map = map__get(initial_map); goto discard_symbol; } curr_map_dso = map__dso(curr_map); @@ -888,7 +889,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, * symbols at this point. */ goto discard_symbol; - } else if (curr_map != initial_map) { + } else if (!RC_CHK_EQUAL(curr_map, initial_map)) { char dso_name[PATH_MAX]; struct dso *ndso; @@ -899,7 +900,8 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, } if (count == 0) { - curr_map = initial_map; + map__zput(curr_map); + curr_map = map__get(initial_map); goto add_symbol; } @@ -913,6 +915,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, kernel_range++); ndso = dso__new(dso_name); + map__zput(curr_map); if (ndso == NULL) return -1; @@ -926,6 +929,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, map__set_mapping_type(curr_map, MAPPING_TYPE__IDENTITY); if (maps__insert(kmaps, curr_map)) { + map__zput(curr_map); dso__put(ndso); return -1; } @@ -936,7 +940,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta, pos->end -= delta; } add_symbol: - if (curr_map != initial_map) { + if (!RC_CHK_EQUAL(curr_map, initial_map)) { struct dso *curr_map_dso = map__dso(curr_map); rb_erase_cached(&pos->rb_node, root); @@ -951,12 +955,12 @@ discard_symbol: symbol__delete(pos); } - if (curr_map != initial_map && + if (!RC_CHK_EQUAL(curr_map, initial_map) && dso->kernel == DSO_SPACE__KERNEL_GUEST && machine__is_default_guest(maps__machine(kmaps))) { dso__set_loaded(map__dso(curr_map)); } - + map__put(curr_map); return count + moved; } -- cgit v1.2.3 From ff0bd79980fffa00c36eb2b9044dbe9cfdf4bb79 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 9 Feb 2024 19:17:45 -0800 Subject: perf maps: Hide maps internals Move the struct into the C file. Add maps__equal to work around exposing the struct for reference count checking. Add accessors for the unwind_libunwind_ops. Move maps_list_node to its only use in symbol.c. Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Cc: K Prateek Nayak Cc: James Clark Cc: Vincent Whitchurch Cc: Alexey Dobriyan Cc: Colin Ian King Cc: Changbin Du Cc: Masami Hiramatsu Cc: Song Liu Cc: Leo Yan Cc: Athira Rajeev Cc: Liam Howlett Cc: Artem Savkov Cc: bpf@vger.kernel.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240210031746.4057262-6-irogers@google.com --- tools/perf/tests/thread-maps-share.c | 8 +-- tools/perf/util/callchain.c | 2 +- tools/perf/util/maps.c | 96 +++++++++++++++++++++++++++++++ tools/perf/util/maps.h | 97 +++----------------------------- tools/perf/util/symbol.c | 10 ++++ tools/perf/util/thread.c | 2 +- tools/perf/util/unwind-libdw.c | 2 +- tools/perf/util/unwind-libunwind-local.c | 2 +- tools/perf/util/unwind-libunwind.c | 7 +-- 9 files changed, 124 insertions(+), 102 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/thread-maps-share.c b/tools/perf/tests/thread-maps-share.c index 7fa6f7c568e2..e9ecd30a5c05 100644 --- a/tools/perf/tests/thread-maps-share.c +++ b/tools/perf/tests/thread-maps-share.c @@ -46,9 +46,9 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(maps)), 4); /* test the maps pointer is shared */ - TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t1))); - TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t2))); - TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t3))); + TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t1))); + TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t2))); + TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t3))); /* * Verify the other leader was created by previous call. @@ -73,7 +73,7 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s other_maps = thread__maps(other); TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(other_maps)), 2); - TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(other_maps, thread__maps(other_leader))); + TEST_ASSERT_VAL("maps don't match", maps__equal(other_maps, thread__maps(other_leader))); /* release thread group */ thread__put(t3); diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 8262f69118db..7517d16c02ec 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -1157,7 +1157,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node * if (al->map == NULL) goto out; } - if (RC_CHK_EQUAL(al->maps, machine__kernel_maps(machine))) { + if (maps__equal(al->maps, machine__kernel_maps(machine))) { if (machine__is_host(machine)) { al->cpumode = PERF_RECORD_MISC_KERNEL; al->level = 'k'; diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c index df0c8041899e..439cefab112a 100644 --- a/tools/perf/util/maps.c +++ b/tools/perf/util/maps.c @@ -6,9 +6,63 @@ #include "dso.h" #include "map.h" #include "maps.h" +#include "rwsem.h" #include "thread.h" #include "ui/ui.h" #include "unwind.h" +#include + +/* + * Locking/sorting note: + * + * Sorting is done with the write lock, iteration and binary searching happens + * under the read lock requiring being sorted. There is a race between sorting + * releasing the write lock and acquiring the read lock for iteration/searching + * where another thread could insert and break the sorting of the maps. In + * practice inserting maps should be rare meaning that the race shouldn't lead + * to live lock. Removal of maps doesn't break being sorted. + */ + +DECLARE_RC_STRUCT(maps) { + struct rw_semaphore lock; + /** + * @maps_by_address: array of maps sorted by their starting address if + * maps_by_address_sorted is true. + */ + struct map **maps_by_address; + /** + * @maps_by_name: optional array of maps sorted by their dso name if + * maps_by_name_sorted is true. + */ + struct map **maps_by_name; + struct machine *machine; +#ifdef HAVE_LIBUNWIND_SUPPORT + void *addr_space; + const struct unwind_libunwind_ops *unwind_libunwind_ops; +#endif + refcount_t refcnt; + /** + * @nr_maps: number of maps_by_address, and possibly maps_by_name, + * entries that contain maps. + */ + unsigned int nr_maps; + /** + * @nr_maps_allocated: number of entries in maps_by_address and possibly + * maps_by_name. + */ + unsigned int nr_maps_allocated; + /** + * @last_search_by_name_idx: cache of last found by name entry's index + * as frequent searches for the same dso name are common. + */ + unsigned int last_search_by_name_idx; + /** @maps_by_address_sorted: is maps_by_address sorted. */ + bool maps_by_address_sorted; + /** @maps_by_name_sorted: is maps_by_name sorted. */ + bool maps_by_name_sorted; + /** @ends_broken: does the map contain a map where end values are unset/unsorted? */ + bool ends_broken; +}; static void check_invariants(const struct maps *maps __maybe_unused) { @@ -118,6 +172,43 @@ static void maps__set_maps_by_name_sorted(struct maps *maps, bool value) RC_CHK_ACCESS(maps)->maps_by_name_sorted = value; } +struct machine *maps__machine(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->machine; +} + +unsigned int maps__nr_maps(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->nr_maps; +} + +refcount_t *maps__refcnt(struct maps *maps) +{ + return &RC_CHK_ACCESS(maps)->refcnt; +} + +#ifdef HAVE_LIBUNWIND_SUPPORT +void *maps__addr_space(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->addr_space; +} + +void maps__set_addr_space(struct maps *maps, void *addr_space) +{ + RC_CHK_ACCESS(maps)->addr_space = addr_space; +} + +const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps) +{ + return RC_CHK_ACCESS(maps)->unwind_libunwind_ops; +} + +void maps__set_unwind_libunwind_ops(struct maps *maps, const struct unwind_libunwind_ops *ops) +{ + RC_CHK_ACCESS(maps)->unwind_libunwind_ops = ops; +} +#endif + static struct rw_semaphore *maps__lock(struct maps *maps) { /* @@ -453,6 +544,11 @@ bool maps__empty(struct maps *maps) return maps__nr_maps(maps) == 0; } +bool maps__equal(struct maps *a, struct maps *b) +{ + return RC_CHK_EQUAL(a, b); +} + int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data), void *data) { bool done = false; diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h index df9dd5a0e3c0..4bcba136ffe5 100644 --- a/tools/perf/util/maps.h +++ b/tools/perf/util/maps.h @@ -3,80 +3,15 @@ #define __PERF_MAPS_H #include -#include #include #include #include -#include "rwsem.h" -#include struct ref_reloc_sym; struct machine; struct map; struct maps; -struct map_list_node { - struct list_head node; - struct map *map; -}; - -static inline struct map_list_node *map_list_node__new(void) -{ - return malloc(sizeof(struct map_list_node)); -} - -/* - * Locking/sorting note: - * - * Sorting is done with the write lock, iteration and binary searching happens - * under the read lock requiring being sorted. There is a race between sorting - * releasing the write lock and acquiring the read lock for iteration/searching - * where another thread could insert and break the sorting of the maps. In - * practice inserting maps should be rare meaning that the race shouldn't lead - * to live lock. Removal of maps doesn't break being sorted. - */ - -DECLARE_RC_STRUCT(maps) { - struct rw_semaphore lock; - /** - * @maps_by_address: array of maps sorted by their starting address if - * maps_by_address_sorted is true. - */ - struct map **maps_by_address; - /** - * @maps_by_name: optional array of maps sorted by their dso name if - * maps_by_name_sorted is true. - */ - struct map **maps_by_name; - struct machine *machine; -#ifdef HAVE_LIBUNWIND_SUPPORT - void *addr_space; - const struct unwind_libunwind_ops *unwind_libunwind_ops; -#endif - refcount_t refcnt; - /** - * @nr_maps: number of maps_by_address, and possibly maps_by_name, - * entries that contain maps. - */ - unsigned int nr_maps; - /** - * @nr_maps_allocated: number of entries in maps_by_address and possibly - * maps_by_name. - */ - unsigned int nr_maps_allocated; - /** - * @last_search_by_name_idx: cache of last found by name entry's index - * as frequent searches for the same dso name are common. - */ - unsigned int last_search_by_name_idx; - /** @maps_by_address_sorted: is maps_by_address sorted. */ - bool maps_by_address_sorted; - /** @maps_by_name_sorted: is maps_by_name sorted. */ - bool maps_by_name_sorted; - /** @ends_broken: does the map contain a map where end values are unset/unsorted? */ - bool ends_broken; -}; - #define KMAP_NAME_LEN 256 struct kmap { @@ -100,36 +35,22 @@ static inline void __maps__zput(struct maps **map) #define maps__zput(map) __maps__zput(&map) +bool maps__equal(struct maps *a, struct maps *b); + /* Iterate over map calling cb for each entry. */ int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data), void *data); /* Iterate over map removing an entry if cb returns true. */ void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data), void *data); -static inline struct machine *maps__machine(struct maps *maps) -{ - return RC_CHK_ACCESS(maps)->machine; -} - -static inline unsigned int maps__nr_maps(const struct maps *maps) -{ - return RC_CHK_ACCESS(maps)->nr_maps; -} - -static inline refcount_t *maps__refcnt(struct maps *maps) -{ - return &RC_CHK_ACCESS(maps)->refcnt; -} +struct machine *maps__machine(const struct maps *maps); +unsigned int maps__nr_maps(const struct maps *maps); +refcount_t *maps__refcnt(struct maps *maps); #ifdef HAVE_LIBUNWIND_SUPPORT -static inline void *maps__addr_space(struct maps *maps) -{ - return RC_CHK_ACCESS(maps)->addr_space; -} - -static inline const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps) -{ - return RC_CHK_ACCESS(maps)->unwind_libunwind_ops; -} +void *maps__addr_space(const struct maps *maps); +void maps__set_addr_space(struct maps *maps, void *addr_space); +const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps); +void maps__set_unwind_libunwind_ops(struct maps *maps, const struct unwind_libunwind_ops *ops); #endif size_t maps__fprintf(struct maps *maps, FILE *fp); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 0785a54e832e..35975189999b 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -63,6 +63,16 @@ struct symbol_conf symbol_conf = { .res_sample = 0, }; +struct map_list_node { + struct list_head node; + struct map *map; +}; + +static struct map_list_node *map_list_node__new(void) +{ + return malloc(sizeof(struct map_list_node)); +} + static enum dso_binary_type binary_type_symtab[] = { DSO_BINARY_TYPE__KALLSYMS, DSO_BINARY_TYPE__GUEST_KALLSYMS, diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 89c47a5098e2..c59ab4d79163 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -383,7 +383,7 @@ static int thread__clone_maps(struct thread *thread, struct thread *parent, bool if (thread__pid(thread) == thread__pid(parent)) return thread__prepare_access(thread); - if (RC_CHK_EQUAL(thread__maps(thread), thread__maps(parent))) { + if (maps__equal(thread__maps(thread), thread__maps(parent))) { pr_debug("broken map groups on thread %d/%d parent %d/%d\n", thread__pid(thread), thread__tid(thread), thread__pid(parent), thread__tid(parent)); diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c index 6013335a8dae..b38d322734b4 100644 --- a/tools/perf/util/unwind-libdw.c +++ b/tools/perf/util/unwind-libdw.c @@ -263,7 +263,7 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg, struct unwind_info *ui, ui_buf = { .sample = data, .thread = thread, - .machine = RC_CHK_ACCESS(thread__maps(thread))->machine, + .machine = maps__machine((thread__maps(thread))), .cb = cb, .arg = arg, .max_stack = max_stack, diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c index dac536e28360..6a5ac0faa6f4 100644 --- a/tools/perf/util/unwind-libunwind-local.c +++ b/tools/perf/util/unwind-libunwind-local.c @@ -706,7 +706,7 @@ static int _unwind__prepare_access(struct maps *maps) { void *addr_space = unw_create_addr_space(&accessors, 0); - RC_CHK_ACCESS(maps)->addr_space = addr_space; + maps__set_addr_space(maps, addr_space); if (!addr_space) { pr_err("unwind: Can't create unwind address space.\n"); return -ENOMEM; diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c index 76cd63de80a8..2728eb4f13ea 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -12,11 +12,6 @@ struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops; struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops; struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops; -static void unwind__register_ops(struct maps *maps, struct unwind_libunwind_ops *ops) -{ - RC_CHK_ACCESS(maps)->unwind_libunwind_ops = ops; -} - int unwind__prepare_access(struct maps *maps, struct map *map, bool *initialized) { const char *arch; @@ -60,7 +55,7 @@ int unwind__prepare_access(struct maps *maps, struct map *map, bool *initialized return 0; } out_register: - unwind__register_ops(maps, ops); + maps__set_unwind_libunwind_ops(maps, ops); err = maps__unwind_libunwind_ops(maps)->prepare_access(maps); if (initialized) -- cgit v1.2.3 From 6f04d664a9fa191e97b54a19f95f2db140554662 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 31 Jan 2024 21:24:16 +0200 Subject: perf test: Enable Symbols test to work with a current module dso The test needs a struct machine and creates one for the current host, but a side-effect is that struct machine has set up kernel maps including module maps. If the 'Symbols' test --dso option specifies a current kernel module, it will already be present as a kernel dso, and a map with kmaps needs to be used otherwise there will be a segfault - see below. For that case, find the existing map and use that. In that case also, the dso is split by section into multiple dsos, so test those dsos also. That in turn, shows up that those dsos have not had overlapping symbols removed, so the test fails. Example: Before: $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko 70: Symbols : --- start --- Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko Segmentation fault (core dumped) After: $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko 70: Symbols : --- start --- Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko Overlapping symbols: 41d30-41fbb l vmx_init 41d30-41fbb g init_module ---- end ---- Symbols: FAILED! Signed-off-by: Adrian Hunter Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240131192416.16387-1-adrian.hunter@intel.com --- tools/perf/tests/symbols.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c index 16e1c5502b09..2fed6d67f50f 100644 --- a/tools/perf/tests/symbols.c +++ b/tools/perf/tests/symbols.c @@ -41,6 +41,30 @@ static void exit_test_info(struct test_info *ti) machine__delete(ti->machine); } +struct dso_map { + struct dso *dso; + struct map *map; +}; + +static int find_map_cb(struct map *map, void *d) +{ + struct dso_map *data = d; + + if (map__dso(map) != data->dso) + return 0; + data->map = map; + return 1; +} + +static struct map *find_module_map(struct machine *machine, struct dso *dso) +{ + struct dso_map data = { .dso = dso }; + + machine__for_each_kernel_map(machine, find_map_cb, &data); + + return data.map; +} + static void get_test_dso_filename(char *filename, size_t max_sz) { if (dso_to_test) @@ -51,6 +75,26 @@ static void get_test_dso_filename(char *filename, size_t max_sz) static int create_map(struct test_info *ti, char *filename, struct map **map_p) { + struct dso *dso = machine__findnew_dso(ti->machine, filename); + + /* + * If 'filename' matches a current kernel module, must use a kernel + * map. Find the one that already exists. + */ + if (dso && dso->kernel) { + *map_p = find_module_map(ti->machine, dso); + dso__put(dso); + if (!*map_p) { + pr_debug("Failed to find map for curent kernel module %s", + filename); + return TEST_FAIL; + } + map__get(*map_p); + return TEST_OK; + } + + dso__put(dso); + /* Create a dummy map at 0x100000 */ *map_p = map__new(ti->machine, 0x100000, 0xffffffff, 0, NULL, PROT_EXEC, 0, NULL, filename, ti->thread); @@ -97,6 +141,26 @@ static int test_dso(struct dso *dso) return ret; } +static int subdivided_dso_cb(struct dso *dso, struct machine *machine __maybe_unused, void *d) +{ + struct dso *text_dso = d; + + if (dso != text_dso && strstarts(dso->short_name, text_dso->short_name)) + if (test_dso(dso) != TEST_OK) + return -1; + + return 0; +} + +static int process_subdivided_dso(struct machine *machine, struct dso *dso) +{ + int ret; + + ret = machine__for_each_dso(machine, subdivided_dso_cb, dso); + + return ret < 0 ? TEST_FAIL : TEST_OK; +} + static int test_file(struct test_info *ti, char *filename) { struct map *map = NULL; @@ -124,6 +188,10 @@ static int test_file(struct test_info *ti, char *filename) } ret = test_dso(dso); + + /* Module dso is split into many dsos by section */ + if (ret == TEST_OK && dso->kernel) + ret = process_subdivided_dso(ti->machine, dso); out_put: map__put(map); -- cgit v1.2.3 From 0aa81428717c88b6f0849944b51b35bc6f613914 Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:25 +0100 Subject: perf testsuite: Add common regex patters Unify perf regexes for checking testing output into a single file to reduce duplicates and prevent errors when editing. This will be used in upcomming patches in shell tests. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-2-mpetlan@redhat.com --- tools/perf/tests/shell/common/patterns.sh | 268 ++++++++++++++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 tools/perf/tests/shell/common/patterns.sh (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/common/patterns.sh b/tools/perf/tests/shell/common/patterns.sh new file mode 100644 index 000000000000..21dab25c7b7f --- /dev/null +++ b/tools/perf/tests/shell/common/patterns.sh @@ -0,0 +1,268 @@ +# SPDX-License-Identifier: GPL-2.0 + +export RE_NUMBER="[0-9\.]+" +# Number +# Examples: +# 123.456 + + +export RE_NUMBER_HEX="[0-9A-Fa-f]+" +# Hexadecimal number +# Examples: +# 1234 +# a58d +# aBcD +# deadbeef + + +export RE_DATE_YYYYMMDD="[0-9]{4}-(?:(?:01|03|05|07|08|10|12)-(?:[0-2][0-9]|3[0-1])|02-[0-2][0-9]|(?:(?:04|06|09|11)-(?:[0-2][0-9]|30)))" +# Date in YYYY-MM-DD form +# Examples: +# 1990-02-29 +# 0015-07-31 +# 2456-12-31 +#! 2012-13-01 +#! 1963-09-31 + + +export RE_TIME="(?:[0-1][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]" +# Time +# Examples: +# 15:12:27 +# 23:59:59 +#! 24:00:00 +#! 11:25:60 +#! 17:60:15 + + +export RE_DATE_TIME="\w+\s+\w+\s+$RE_NUMBER\s+$RE_TIME\s+$RE_NUMBER" +# Time and date +# Examples: +# Wed Feb 12 10:46:26 2020 +# Mon Mar 2 13:27:06 2020 +#! St úno 12 10:57:21 CET 2020 +#! Po úno 14 15:17:32 2010 + + +export RE_ADDRESS="0x$RE_NUMBER_HEX" +# Memory address +# Examples: +# 0x123abc +# 0xffffffff9abe8ae8 +# 0x0 + + +export RE_ADDRESS_NOT_NULL="0x[0-9A-Fa-f]*[1-9A-Fa-f]+[0-9A-Fa-f]*" +# Memory address (not NULL) +# Examples: +# 0xffffffff9abe8ae8 +#! 0x0 +#! 0x0000000000000000 + +export RE_PROCESS_PID="[^\/]+\/\d+" +# A process with PID +# Example: +# sleep/4102 +# test_overhead./866185 +# in:imjournal/1096 +# random#$& test/866607 + +export RE_EVENT_ANY="[\w\-\:\/_=,]+" +# Name of any event (universal) +# Examples: +# cpu-cycles +# cpu/event=12,umask=34/ +# r41e1 +# nfs:nfs_getattr_enter + + +export RE_EVENT="[\w\-:_]+" +# Name of an usual event +# Examples: +# cpu-cycles + + +export RE_EVENT_RAW="r$RE_NUMBER_HEX" +# Specification of a raw event +# Examples: +# r41e1 +# r1a + + +export RE_EVENT_CPU="cpu/(\w+=$RE_NUMBER_HEX,?)+/p*" +# Specification of a CPU event +# Examples: +# cpu/event=12,umask=34/pp + + +export RE_EVENT_UNCORE="uncore/[\w_]+/" +# Specification of an uncore event +# Examples: +# uncore/qhl_request_local_reads/ + + +export RE_EVENT_SUBSYSTEM="[\w\-]+:[\w\-]+" +# Name of an event from subsystem +# Examples: +# ext4:ext4_ordered_write_end +# sched:sched_switch + + +export RE_FILE_NAME="[\w\+\.-]+" +# A filename +# Examples: +# libstdc++.so.6 +#! some/path + + +export RE_PATH_ABSOLUTE="(?:\/$RE_FILE_NAME)+" +# A full filepath +# Examples: +# /usr/lib64/somelib.so.5.4.0 +# /lib/modules/4.3.0-rc5/kernel/fs/xfs/xfs.ko +# /usr/bin/mv +#! some/relative/path +#! ./some/relative/path + + +export RE_PATH="(?:$RE_FILE_NAME)?$RE_PATH_ABSOLUTE" +# A filepath +# Examples: +# /usr/lib64/somelib.so.5.4.0 +# /lib/modules/4.3.0-rc5/kernel/fs/xfs/xfs.ko +# ./.emacs +# src/fs/file.c + + +export RE_DSO="(?:$RE_PATH_ABSOLUTE(?: \(deleted\))?|\[kernel\.kallsyms\]|\[unknown\]|\[vdso\]|\[kernel\.vmlinux\][\.\w]*)" +# A DSO name in various result tables +# Examples: +# /usr/lib64/somelib.so.5.4.0 +# /usr/bin/somebinart (deleted) +# /lib/modules/4.3.0-rc5/kernel/fs/xfs/xfs.ko +# [kernel.kallsyms] +# [kernel.vmlinux] +# [vdso] +# [unknown] + + +export RE_LINE_COMMENT="^#.*" +# A comment line +# Examples: +# # Started on Thu Sep 10 11:43:00 2015 + + +export RE_LINE_EMPTY="^\s*$" +# An empty line with possible whitespaces +# Examples: +# + + +export RE_LINE_RECORD1="^\[\s+perf\s+record:\s+Woken up $RE_NUMBER times? to write data\s+\].*$" +# The first line of perf-record "OK" output +# Examples: +# [ perf record: Woken up 1 times to write data ] + + +export RE_LINE_RECORD2="^\[\s+perf\s+record:\s+Captured and wrote $RE_NUMBER\s*MB\s+(?:[\w\+\.-]*(?:$RE_PATH)?\/)?perf\.data(?:\.\d+)?\s*\(~?$RE_NUMBER samples\)\s+\].*$" +# The second line of perf-record "OK" output +# Examples: +# [ perf record: Captured and wrote 0.405 MB perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB perf.data (~109 samples) ] +# [ perf record: Captured and wrote 0.405 MB /some/temp/dir/perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB ./perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB ./perf.data.3 (109 samples) ] + + +export RE_LINE_RECORD2_TOLERANT="^\[\s+perf\s+record:\s+Captured and wrote $RE_NUMBER\s*MB\s+(?:[\w\+\.-]*(?:$RE_PATH)?\/)?perf\.data(?:\.\d+)?\s*(?:\(~?$RE_NUMBER samples\))?\s+\].*$" +# The second line of perf-record "OK" output, even no samples is OK here +# Examples: +# [ perf record: Captured and wrote 0.405 MB perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB perf.data (~109 samples) ] +# [ perf record: Captured and wrote 0.405 MB /some/temp/dir/perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB ./perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB ./perf.data.3 (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB perf.data ] + + +export RE_LINE_RECORD2_TOLERANT_FILENAME="^\[\s+perf\s+record:\s+Captured and wrote $RE_NUMBER\s*MB\s+(?:[\w\+\.-]*(?:$RE_PATH)?\/)?perf\w*\.data(?:\.\d+)?\s*\(~?$RE_NUMBER samples\)\s+\].*$" +# The second line of perf-record "OK" output +# Examples: +# [ perf record: Captured and wrote 0.405 MB perf.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB perf_ls.data (~109 samples) ] +# [ perf record: Captured and wrote 0.405 MB perf_aNyCaSe.data (109 samples) ] +# [ perf record: Captured and wrote 0.405 MB ./perfdata.data.3 (109 samples) ] +#! [ perf record: Captured and wrote 0.405 MB /some/temp/dir/my_own.data (109 samples) ] +#! [ perf record: Captured and wrote 0.405 MB ./UPPERCASE.data (109 samples) ] +#! [ perf record: Captured and wrote 0.405 MB ./aNyKiNDoF.data.3 (109 samples) ] +#! [ perf record: Captured and wrote 0.405 MB perf.data ] + + +export RE_LINE_TRACE_FULL="^\s*$RE_NUMBER\s*\(\s*$RE_NUMBER\s*ms\s*\):\s*$RE_PROCESS_PID\s+.*\)\s+=\s+(:?\-?$RE_NUMBER|0x$RE_NUMBER_HEX).*$" +# A line of perf-trace output +# Examples: +# 0.115 ( 0.005 ms): sleep/4102 open(filename: 0xd09e2ab2, flags: CLOEXEC ) = 3 +# 0.157 ( 0.005 ms): sleep/4102 mmap(len: 3932736, prot: EXEC|READ, flags: PRIVATE|DENYWRITE, fd: 3 ) = 0x7f89d0605000 +#! 0.115 ( 0.005 ms): sleep/4102 open(filename: 0xd09e2ab2, flags: CLOEXEC ) = + +export RE_LINE_TRACE_ONE_PROC="^\s*$RE_NUMBER\s*\(\s*$RE_NUMBER\s*ms\s*\):\s*\w+\(.*\)\s+=\s+(?:\-?$RE_NUMBER|0x$RE_NUMBER_HEX).*$" +# A line of perf-trace output +# Examples: +# 0.115 ( 0.005 ms): open(filename: 0xd09e2ab2, flags: CLOEXEC ) = 3 +# 0.157 ( 0.005 ms): mmap(len: 3932736, prot: EXEC|READ, flags: PRIVATE|DENYWRITE, fd: 3 ) = 0x7f89d0605000 +#! 0.115 ( 0.005 ms): open(filename: 0xd09e2ab2, flags: CLOEXEC ) = + +export RE_LINE_TRACE_CONTINUED="^\s*(:?$RE_NUMBER|\?)\s*\(\s*($RE_NUMBER\s*ms\s*)?\):\s*($RE_PROCESS_PID\s*)?\.\.\.\s*\[continued\]:\s+\w+\(\).*\s+=\s+(?:\-?$RE_NUMBER|0x$RE_NUMBER_HEX).*$" +# A line of perf-trace output +# Examples: +# 0.000 ( 0.000 ms): ... [continued]: nanosleep()) = 0 +# 0.000 ( 0.000 ms): ... [continued]: nanosleep()) = 0x00000000 +# ? ( ): packagekitd/94838 ... [continued]: poll()) = 0 (Timeout) +#! 0.000 ( 0.000 ms): ... [continued]: nanosleep()) = + +export RE_LINE_TRACE_UNFINISHED="^\s*$RE_NUMBER\s*\(\s*\):\s*$RE_PROCESS_PID\s+.*\)\s+\.\.\.\s*$" +# A line of perf-trace output +# Examples: +# 901.040 ( ): in:imjournal/1096 ppoll(ufds: 0x7f701a5adb70, nfds: 1, tsp: 0x7f701a5adaf0, sigsetsize: 8) ... +# 613.727 ( ): gmain/1099 poll(ufds: 0x56248f6b64b0, nfds: 2, timeout_msecs: 3996) ... + +export RE_LINE_TRACE_SUMMARY_HEADER="\s*syscall\s+calls\s+(?:errors\s+)?total\s+min\s+avg\s+max\s+stddev" +# A header of a perf-trace summary table +# Example: +# syscall calls total min avg max stddev +# syscall calls errors total min avg max stddev + + +export RE_LINE_TRACE_SUMMARY_CONTENT="^\s*\w+\s+(?:$RE_NUMBER\s+){5,6}$RE_NUMBER%" +# A line of a perf-trace summary table +# Example: +# open 3 0.017 0.005 0.006 0.007 10.90% +# openat 2 0 0.017 0.008 0.009 0.010 12.29% + + +export RE_LINE_REPORT_CONTENT="^\s+$RE_NUMBER%\s+\w+\s+\S+\s+\S+\s+\S+" # FIXME +# A line from typicap perf report --stdio output +# Example: +# 100.00% sleep [kernel.vmlinux] [k] syscall_return_slowpath + + +export RE_TASK="\s+[\w~\/ \.\+:#-]+(?:\[-1(?:\/\d+)?\]|\[\d+(?:\/\d+)?\])" +# A name of a task used for perf sched timehist -s +# Example: +# sleep[62755] +# runtest.sh[62762] +# gmain[705/682] +# xfsaild/dm-0[495] +# kworker/u8:1-ev[62714] +# :-1[-1/62756] +# :-1[-1] +# :-1[62756] + + +export RE_SEGFAULT=".*(?:Segmentation\sfault|SIGSEGV|\score\s|dumped|segfault).*" +# Possible variations of the segfault message +# Example: +# /bin/bash: line 1: 32 Segmentation fault timeout 15s +# Segmentation fault (core dumped) +# Program terminated with signal SIGSEGV +#! WARNING: 12323431 isn't a 'cpu_core', please use a CPU list in the 'cpu_core' range (0-15) -- cgit v1.2.3 From 451af6a790b4b6f690cb2a0b8e7a6e0591528524 Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:26 +0100 Subject: perf testsuite: Add common setting for shell tests Add settings defining sample commands later shared by shell tests. This adds the possibility to globally adjust the default values for the whole testsuite. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-3-mpetlan@redhat.com --- tools/perf/tests/shell/common/settings.sh | 79 +++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tools/perf/tests/shell/common/settings.sh (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/common/settings.sh b/tools/perf/tests/shell/common/settings.sh new file mode 100644 index 000000000000..361641dbaaad --- /dev/null +++ b/tools/perf/tests/shell/common/settings.sh @@ -0,0 +1,79 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# settings.sh +# Author: Michael Petlan +# +# Description: +# +# This file contains global settings for the whole testsuite. +# Its purpose is to make it easier when it is necessary i.e. to +# change the usual sample command which is used in all of the tests +# in many files. +# +# This file is intended to be sourced in the tests. +# + +#### which perf to use in the testing +export CMD_PERF=${CMD_PERF:-`which perf`} + +#### basic programs examinated by perf +export CMD_BASIC_SLEEP="sleep 0.1" +export CMD_QUICK_SLEEP="sleep 0.01" +export CMD_LONGER_SLEEP="sleep 2" +export CMD_DOUBLE_LONGER_SLEEP="sleep 4" +export CMD_VERY_LONG_SLEEP="sleep 30" +export CMD_SIMPLE="true" + +#### testsuite run mode +# define constants: +export RUNMODE_BASIC=0 +export RUNMODE_STANDARD=1 +export RUNMODE_EXPERIMENTAL=2 +# default runmode is STANDARD +export PERFTOOL_TESTSUITE_RUNMODE=${PERFTOOL_TESTSUITE_RUNMODE:-$RUNMODE_STANDARD} + +#### common settings +export TESTLOG_VERBOSITY=${TESTLOG_VERBOSITY:-2} +export TESTLOG_FORCE_COLOR=${TESTLOG_FORCE_COLOR:-n} +export TESTLOG_ERR_MSG_MAX_LINES=${TESTLOG_ERR_MSG_MAX_LINES:-20} +export TESTLOG_CLEAN=${TESTLOG_CLEAN:-y} + +#### other environment-related settings +export TEST_IGNORE_MISSING_PMU=${TEST_IGNORE_MISSING_PMU:-n} + +#### clear locale +export LC_ALL=C + +#### colors +if [ -t 1 -o "$TESTLOG_FORCE_COLOR" = "yes" ]; then + export MPASS="\e[32m" + export MALLPASS="\e[1;32m" + export MFAIL="\e[31m" + export MALLFAIL="\e[1;31m" + export MWARN="\e[1;35m" + export MSKIP="\e[33m" + export MHIGH="\e[1;33m" + export MEND="\e[m" +else + export MPASS="" + export MALLPASS="" + export MFAIL="" + export MALLFAIL="" + export MWARN="" + export MSKIP="" + export MHIGH="" + export MEND="" +fi + + +#### test parametrization +if [ ! -d ./common ]; then + # set parameters based on runmode + if [ -f ../common/parametrization.$PERFTOOL_TESTSUITE_RUNMODE.sh ]; then + . ../common/parametrization.$PERFTOOL_TESTSUITE_RUNMODE.sh + fi + # if some parameters haven't been set until now, set them to default + if [ -f ../common/parametrization.sh ]; then + . ../common/parametrization.sh + fi +fi -- cgit v1.2.3 From e3425864a9e4dfad7bfb2de62b61fb4f72629aa6 Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:27 +0100 Subject: perf testsuite: Add initialization script for shell tests Initialize reporting and logging functions that unifies formatting of the test output used for shell tests. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-4-mpetlan@redhat.com --- tools/perf/tests/shell/common/init.sh | 117 ++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 tools/perf/tests/shell/common/init.sh (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/common/init.sh b/tools/perf/tests/shell/common/init.sh new file mode 100644 index 000000000000..aadeaf782e03 --- /dev/null +++ b/tools/perf/tests/shell/common/init.sh @@ -0,0 +1,117 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# init.sh +# Author: Michael Petlan +# +# Description: +# +# This file should be used for initialization of basic functions +# for checking, reporting results etc. +# +# + + +. ../common/settings.sh +. ../common/patterns.sh + +THIS_TEST_NAME=`basename $0 .sh` + +_echo() +{ + test "$TESTLOG_VERBOSITY" -ne 0 && echo -e "$@" +} + +print_results() +{ + PERF_RETVAL="$1"; shift + CHECK_RETVAL="$1"; shift + FAILURE_REASON="" + TASK_COMMENT="$@" + if [ $PERF_RETVAL -eq 0 -a $CHECK_RETVAL -eq 0 ]; then + _echo "$MPASS-- [ PASS ] --$MEND $TEST_NAME :: $THIS_TEST_NAME :: $TASK_COMMENT" + return 0 + else + if [ $PERF_RETVAL -ne 0 ]; then + FAILURE_REASON="command exitcode" + fi + if [ $CHECK_RETVAL -ne 0 ]; then + test -n "$FAILURE_REASON" && FAILURE_REASON="$FAILURE_REASON + " + FAILURE_REASON="$FAILURE_REASON""output regexp parsing" + fi + _echo "$MFAIL-- [ FAIL ] --$MEND $TEST_NAME :: $THIS_TEST_NAME :: $TASK_COMMENT ($FAILURE_REASON)" + return 1 + fi +} + +print_overall_results() +{ + RETVAL="$1"; shift + if [ $RETVAL -eq 0 ]; then + _echo "$MALLPASS## [ PASS ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY" + else + _echo "$MALLFAIL## [ FAIL ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY :: $RETVAL failures found" + fi + return $RETVAL +} + +print_testcase_skipped() +{ + TASK_COMMENT="$@" + _echo "$MSKIP-- [ SKIP ] --$MEND $TEST_NAME :: $THIS_TEST_NAME :: $TASK_COMMENT :: testcase skipped" + return 0 +} + +print_overall_skipped() +{ + _echo "$MSKIP## [ SKIP ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME :: testcase skipped" + return 0 +} + +print_warning() +{ + WARN_COMMENT="$@" + _echo "$MWARN-- [ WARN ] --$MEND $TEST_NAME :: $THIS_TEST_NAME :: $WARN_COMMENT" + return 0 +} + +# this function should skip a testcase if the testsuite is not run in +# a runmode that fits the testcase --> if the suite runs in BASIC mode +# all STANDARD and EXPERIMENTAL testcases will be skipped; if the suite +# runs in STANDARD mode, all EXPERIMENTAL testcases will be skipped and +# if the suite runs in EXPERIMENTAL mode, nothing is skipped +consider_skipping() +{ + TESTCASE_RUNMODE="$1" + # the runmode of a testcase needs to be at least the current suite's runmode + if [ $PERFTOOL_TESTSUITE_RUNMODE -lt $TESTCASE_RUNMODE ]; then + print_overall_skipped + exit 0 + fi +} + +detect_baremetal() +{ + # return values: + # 0 = bare metal + # 1 = virtualization detected + # 2 = unknown state + VIRT=`systemd-detect-virt 2>/dev/null` + test $? -eq 127 && return 2 + test "$VIRT" = "none" +} + +detect_intel() +{ + # return values: + # 0 = is Intel + # 1 = is not Intel or unknown + grep "vendor_id" < /proc/cpuinfo | grep -q "GenuineIntel" +} + +detect_amd() +{ + # return values: + # 0 = is AMD + # 1 = is not AMD or unknown + grep "vendor_id" < /proc/cpuinfo | grep -q "AMD" +} -- cgit v1.2.3 From c8eb2a9ff8b30957d90c941cb4f0caac705ceffb Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:28 +0100 Subject: perf testsuite: Add test case for perf probe Add new perf probe test case that acts as an entry element in perf test list. Runs multiple subtests from directory "base_probe", which will be added in incomming patches and can be expanded without further editing. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-5-mpetlan@redhat.com --- tools/perf/tests/shell/perftool-testsuite_probe.sh | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100755 tools/perf/tests/shell/perftool-testsuite_probe.sh (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/perftool-testsuite_probe.sh b/tools/perf/tests/shell/perftool-testsuite_probe.sh new file mode 100755 index 000000000000..a0fec33a0358 --- /dev/null +++ b/tools/perf/tests/shell/perftool-testsuite_probe.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# perftool-testsuite_probe +# SPDX-License-Identifier: GPL-2.0 + +test -d "$(dirname "$0")/base_probe" || exit 2 +cd "$(dirname "$0")/base_probe" || exit 2 +status=0 + +PERFSUITE_RUN_DIR=$(mktemp -d /tmp/"$(basename "$0" .sh)".XXX) +export PERFSUITE_RUN_DIR + +for testcase in setup.sh test_*; do # skip setup.sh if not present or not executable + test -x "$testcase" || continue + ./"$testcase" + (( status += $? )) +done + +if ! [ "$PERFTEST_KEEP_LOGS" = "y" ]; then + rm -rf "$PERFSUITE_RUN_DIR" +fi + +test $status -ne 0 && exit 1 +exit 0 -- cgit v1.2.3 From 61d348f1e96fe11ba7d3714bcc700d8fd71aa17e Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:29 +0100 Subject: perf testsuite: Add common output checking helpers As a form of validation, it is a common practice to check the outputs of commands whether they contain expected patterns or match a certain regex. Add helpers for verifying that all regexes are found in the output, that all lines match any pattern from a set and that a certain expression is not present in the output. In verbose mode these helpers log mismatches for easier failure investigation. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-6-mpetlan@redhat.com --- .../tests/shell/common/check_all_lines_matched.pl | 39 ++++++++++++++++++++++ .../tests/shell/common/check_all_patterns_found.pl | 34 +++++++++++++++++++ .../tests/shell/common/check_no_patterns_found.pl | 34 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100755 tools/perf/tests/shell/common/check_all_lines_matched.pl create mode 100755 tools/perf/tests/shell/common/check_all_patterns_found.pl create mode 100755 tools/perf/tests/shell/common/check_no_patterns_found.pl (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/common/check_all_lines_matched.pl b/tools/perf/tests/shell/common/check_all_lines_matched.pl new file mode 100755 index 000000000000..fded48959a3f --- /dev/null +++ b/tools/perf/tests/shell/common/check_all_lines_matched.pl @@ -0,0 +1,39 @@ +#!/usr/bin/perl +# SPDX-License-Identifier: GPL-2.0 + +@regexps = @ARGV; + +$max_printed_lines = 20; +$max_printed_lines = $ENV{TESTLOG_ERR_MSG_MAX_LINES} if (defined $ENV{TESTLOG_ERR_MSG_MAX_LINES}); + +$quiet = 1; +$quiet = 0 if (defined $ENV{TESTLOG_VERBOSITY} && $ENV{TESTLOG_VERBOSITY} ge 2); + +$passed = 1; +$lines_printed = 0; + +while () +{ + s/\n//; + + $line_matched = 0; + for $r (@regexps) + { + if (/$r/) + { + $line_matched = 1; + last; + } + } + + unless ($line_matched) + { + if ($lines_printed++ < $max_printed_lines) + { + print "Line did not match any pattern: \"$_\"\n" unless $quiet; + } + $passed = 0; + } +} + +exit ($passed == 0); diff --git a/tools/perf/tests/shell/common/check_all_patterns_found.pl b/tools/perf/tests/shell/common/check_all_patterns_found.pl new file mode 100755 index 000000000000..11bdf1d3460a --- /dev/null +++ b/tools/perf/tests/shell/common/check_all_patterns_found.pl @@ -0,0 +1,34 @@ +#!/usr/bin/perl +# SPDX-License-Identifier: GPL-2.0 + +@regexps = @ARGV; + +$quiet = 1; +$quiet = 0 if (defined $ENV{TESTLOG_VERBOSITY} && $ENV{TESTLOG_VERBOSITY} ge 2); + +%found = (); +$passed = 1; + +while () +{ + s/\n//; + + for $r (@regexps) + { + if (/$r/) + { + $found{$r} = 1; # FIXME: maybe add counters -- how many times was the regexp matched + } + } +} + +for $r (@regexps) +{ + unless (exists $found{$r}) + { + print "Regexp not found: \"$r\"\n" unless $quiet; + $passed = 0; + } +} + +exit ($passed == 0); diff --git a/tools/perf/tests/shell/common/check_no_patterns_found.pl b/tools/perf/tests/shell/common/check_no_patterns_found.pl new file mode 100755 index 000000000000..770999e87a5f --- /dev/null +++ b/tools/perf/tests/shell/common/check_no_patterns_found.pl @@ -0,0 +1,34 @@ +#!/usr/bin/perl +# SPDX-License-Identifier: GPL-2.0 + +@regexps = @ARGV; + +$quiet = 1; +$quiet = 0 if (defined $ENV{TESTLOG_VERBOSITY} && $ENV{TESTLOG_VERBOSITY} ge 2); + +%found = (); +$passed = 1; + +while () +{ + s/\n//; + + for $r (@regexps) + { + if (/$r/) + { + $found{$r} = 1; + } + } +} + +for $r (@regexps) +{ + if (exists $found{$r}) + { + print "Regexp found: \"$r\"\n" unless $quiet; + $passed = 0; + } +} + +exit ($passed == 0); -- cgit v1.2.3 From e7d759f31ca295d589f7420719c311870bb3166f Mon Sep 17 00:00:00 2001 From: Veronika Molnarova Date: Thu, 15 Feb 2024 12:02:30 +0100 Subject: perf testsuite: Add test for kprobe handling Test perf interface to kprobes: listing, adding and removing probes. It is run as a part of perftool-testsuite_probe test case. Signed-off-by: Veronika Molnarova Signed-off-by: Michael Petlan Cc: kjain@linux.ibm.com Cc: atrajeev@linux.vnet.ibm.com Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240215110231.15385-7-mpetlan@redhat.com --- tools/perf/tests/shell/base_probe/settings.sh | 48 ++++ .../tests/shell/base_probe/test_adding_kernel.sh | 278 +++++++++++++++++++++ 2 files changed, 326 insertions(+) create mode 100644 tools/perf/tests/shell/base_probe/settings.sh create mode 100755 tools/perf/tests/shell/base_probe/test_adding_kernel.sh (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/base_probe/settings.sh b/tools/perf/tests/shell/base_probe/settings.sh new file mode 100644 index 000000000000..123621c7f95e --- /dev/null +++ b/tools/perf/tests/shell/base_probe/settings.sh @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# settings.sh of perf_probe test +# Author: Michael Petlan +# Author: Masami Hiramatsu +# + +export TEST_NAME="perf_probe" + +export MY_ARCH=`arch` + +if [ -n "$PERFSUITE_RUN_DIR" ]; then + # when $PERFSUITE_RUN_DIR is set to something, all the logs and temp files will be placed there + # --> the $PERFSUITE_RUN_DIR/perf_something/examples and $PERFSUITE_RUN_DIR/perf_something/logs + # dirs will be used for that + export PERFSUITE_RUN_DIR=`readlink -f $PERFSUITE_RUN_DIR` + export CURRENT_TEST_DIR="$PERFSUITE_RUN_DIR/$TEST_NAME" + export MAKE_TARGET_DIR="$CURRENT_TEST_DIR/examples" + test -d "$MAKE_TARGET_DIR" || mkdir -p "$MAKE_TARGET_DIR" + export LOGS_DIR="$PERFSUITE_RUN_DIR/$TEST_NAME/logs" + test -d "$LOGS_DIR" || mkdir -p "$LOGS_DIR" +else + # when $PERFSUITE_RUN_DIR is not set, logs will be placed here + export CURRENT_TEST_DIR="." + export LOGS_DIR="." +fi + +check_kprobes_available() +{ + test -e /sys/kernel/debug/tracing/kprobe_events +} + +check_uprobes_available() +{ + test -e /sys/kernel/debug/tracing/uprobe_events +} + +clear_all_probes() +{ + echo 0 > /sys/kernel/debug/tracing/events/enable + check_kprobes_available && echo > /sys/kernel/debug/tracing/kprobe_events + check_uprobes_available && echo > /sys/kernel/debug/tracing/uprobe_events +} + +check_sdt_support() +{ + $CMD_PERF list sdt | grep sdt > /dev/null 2> /dev/null +} diff --git a/tools/perf/tests/shell/base_probe/test_adding_kernel.sh b/tools/perf/tests/shell/base_probe/test_adding_kernel.sh new file mode 100755 index 000000000000..a5d707efad85 --- /dev/null +++ b/tools/perf/tests/shell/base_probe/test_adding_kernel.sh @@ -0,0 +1,278 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# +# test_adding_kernel of perf_probe test +# Author: Masami Hiramatsu +# Author: Michael Petlan +# +# Description: +# +# This test tests adding of probes, their correct listing +# and removing. +# + +# include working environment +. ../common/init.sh +. ./settings.sh + +# shellcheck disable=SC2034 # the variable is later used after the working environment is included +THIS_TEST_NAME=`basename $0 .sh` +TEST_RESULT=0 + +TEST_PROBE=${TEST_PROBE:-"inode_permission"} + +check_kprobes_available +if [ $? -ne 0 ]; then + print_overall_skipped + exit 0 +fi + + +### basic probe adding + +for opt in "" "-a" "--add"; do + clear_all_probes + $CMD_PERF probe $opt $TEST_PROBE 2> $LOGS_DIR/adding_kernel_add$opt.err + PERF_EXIT_CODE=$? + + ../common/check_all_patterns_found.pl "Added new events?:" "probe:$TEST_PROBE" "on $TEST_PROBE" < $LOGS_DIR/adding_kernel_add$opt.err + CHECK_EXIT_CODE=$? + + print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "adding probe $TEST_PROBE :: $opt" + (( TEST_RESULT += $? )) +done + + +### listing added probe :: perf list + +# any added probes should appear in perf-list output +$CMD_PERF list probe:\* > $LOGS_DIR/adding_kernel_list.log +PERF_EXIT_CODE=$? + +../common/check_all_lines_matched.pl "$RE_LINE_EMPTY" "List of pre-defined events" "probe:${TEST_PROBE}(?:_\d+)?\s+\[Tracepoint event\]" "Metric Groups:" < $LOGS_DIR/adding_kernel_list.log +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "listing added probe :: perf list" +(( TEST_RESULT += $? )) + + +### listing added probe :: perf probe -l + +# '-l' should list all the added probes as well +$CMD_PERF probe -l > $LOGS_DIR/adding_kernel_list-l.log +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "\s*probe:${TEST_PROBE}(?:_\d+)?\s+\(on ${TEST_PROBE}(?:[:\+]$RE_NUMBER_HEX)?@.+\)" < $LOGS_DIR/adding_kernel_list-l.log +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "listing added probe :: perf probe -l" +(( TEST_RESULT += $? )) + + +### using added probe + +$CMD_PERF stat -e probe:$TEST_PROBE\* -o $LOGS_DIR/adding_kernel_using_probe.log -- cat /proc/uptime > /dev/null +PERF_EXIT_CODE=$? + +REGEX_STAT_HEADER="\s*Performance counter stats for \'cat /proc/uptime\':" +REGEX_STAT_VALUES="\s*\d+\s+probe:$TEST_PROBE" +# the value should be greater than 1 +REGEX_STAT_VALUE_NONZERO="\s*[1-9][0-9]*\s+probe:$TEST_PROBE" +REGEX_STAT_TIME="\s*$RE_NUMBER\s+seconds (?:time elapsed|user|sys)" +../common/check_all_lines_matched.pl "$REGEX_STAT_HEADER" "$REGEX_STAT_VALUES" "$REGEX_STAT_TIME" "$RE_LINE_COMMENT" "$RE_LINE_EMPTY" < $LOGS_DIR/adding_kernel_using_probe.log +CHECK_EXIT_CODE=$? +../common/check_all_patterns_found.pl "$REGEX_STAT_HEADER" "$REGEX_STAT_VALUE_NONZERO" "$REGEX_STAT_TIME" < $LOGS_DIR/adding_kernel_using_probe.log +(( CHECK_EXIT_CODE += $? )) + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "using added probe" +(( TEST_RESULT += $? )) + + +### removing added probe + +# '-d' should remove the probe +$CMD_PERF probe -d $TEST_PROBE\* 2> $LOGS_DIR/adding_kernel_removing.err +PERF_EXIT_CODE=$? + +../common/check_all_lines_matched.pl "Removed event: probe:$TEST_PROBE" < $LOGS_DIR/adding_kernel_removing.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "deleting added probe" +(( TEST_RESULT += $? )) + + +### listing removed probe + +# removed probes should NOT appear in perf-list output +$CMD_PERF list probe:\* > $LOGS_DIR/adding_kernel_list_removed.log +PERF_EXIT_CODE=$? + +../common/check_all_lines_matched.pl "$RE_LINE_EMPTY" "List of pre-defined events" "Metric Groups:" < $LOGS_DIR/adding_kernel_list_removed.log +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "listing removed probe (should NOT be listed)" +(( TEST_RESULT += $? )) + + +### dry run + +# the '-n' switch should run it in dry mode +$CMD_PERF probe -n --add $TEST_PROBE 2> $LOGS_DIR/adding_kernel_dryrun.err +PERF_EXIT_CODE=$? + +# check for the output (should be the same as usual) +../common/check_all_patterns_found.pl "Added new events?:" "probe:$TEST_PROBE" "on $TEST_PROBE" < $LOGS_DIR/adding_kernel_dryrun.err +CHECK_EXIT_CODE=$? + +# check that no probe was added in real +! ( $CMD_PERF probe -l | grep "probe:$TEST_PROBE" ) +(( CHECK_EXIT_CODE += $? )) + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "dry run :: adding probe" +(( TEST_RESULT += $? )) + + +### force-adding probes + +# when using '--force' a probe should be added even if it is already there +$CMD_PERF probe --add $TEST_PROBE 2> $LOGS_DIR/adding_kernel_forceadd_01.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "Added new events?:" "probe:$TEST_PROBE" "on $TEST_PROBE" < $LOGS_DIR/adding_kernel_forceadd_01.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "force-adding probes :: first probe adding" +(( TEST_RESULT += $? )) + +# adding existing probe without '--force' should fail +! $CMD_PERF probe --add $TEST_PROBE 2> $LOGS_DIR/adding_kernel_forceadd_02.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "Error: event \"$TEST_PROBE\" already exists." "Error: Failed to add events." < $LOGS_DIR/adding_kernel_forceadd_02.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "force-adding probes :: second probe adding (without force)" +(( TEST_RESULT += $? )) + +# adding existing probe with '--force' should pass +NO_OF_PROBES=`$CMD_PERF probe -l | wc -l` +$CMD_PERF probe --force --add $TEST_PROBE 2> $LOGS_DIR/adding_kernel_forceadd_03.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "Added new events?:" "probe:${TEST_PROBE}_${NO_OF_PROBES}" "on $TEST_PROBE" < $LOGS_DIR/adding_kernel_forceadd_03.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "force-adding probes :: second probe adding (with force)" +(( TEST_RESULT += $? )) + + +### using doubled probe + +# since they are the same, they should produce the same results +$CMD_PERF stat -e probe:$TEST_PROBE -e probe:${TEST_PROBE}_${NO_OF_PROBES} -x';' -o $LOGS_DIR/adding_kernel_using_two.log -- bash -c 'cat /proc/cpuinfo > /dev/null' +PERF_EXIT_CODE=$? + +REGEX_LINE="$RE_NUMBER;+probe:${TEST_PROBE}_?(?:$NO_OF_PROBES)?;$RE_NUMBER;$RE_NUMBER" +../common/check_all_lines_matched.pl "$REGEX_LINE" "$RE_LINE_EMPTY" "$RE_LINE_COMMENT" < $LOGS_DIR/adding_kernel_using_two.log +CHECK_EXIT_CODE=$? + +VALUE_1=`grep "$TEST_PROBE;" $LOGS_DIR/adding_kernel_using_two.log | awk -F';' '{print $1}'` +VALUE_2=`grep "${TEST_PROBE}_${NO_OF_PROBES};" $LOGS_DIR/adding_kernel_using_two.log | awk -F';' '{print $1}'` + +test $VALUE_1 -eq $VALUE_2 +(( CHECK_EXIT_CODE += $? )) + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "using doubled probe" + + +### removing multiple probes + +# using wildcards should remove all matching probes +$CMD_PERF probe --del \* 2> $LOGS_DIR/adding_kernel_removing_wildcard.err +PERF_EXIT_CODE=$? + +../common/check_all_lines_matched.pl "Removed event: probe:$TEST_PROBE" "Removed event: probe:${TEST_PROBE}_1" < $LOGS_DIR/adding_kernel_removing_wildcard.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "removing multiple probes" +(( TEST_RESULT += $? )) + + +### wildcard adding support + +$CMD_PERF probe -nf --max-probes=512 -a 'vfs_* $params' 2> $LOGS_DIR/adding_kernel_adding_wildcard.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "probe:vfs_mknod" "probe:vfs_create" "probe:vfs_rmdir" "probe:vfs_link" "probe:vfs_write" < $LOGS_DIR/adding_kernel_adding_wildcard.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "wildcard adding support" +(( TEST_RESULT += $? )) + + +### non-existing variable + +# perf probe should survive a non-existing variable probing attempt +{ $CMD_PERF probe 'vfs_read somenonexistingrandomstuffwhichisalsoprettylongorevenlongertoexceed64' ; } 2> $LOGS_DIR/adding_kernel_nonexisting.err +PERF_EXIT_CODE=$? + +# the exitcode should not be 0 or segfault +test $PERF_EXIT_CODE -ne 139 -a $PERF_EXIT_CODE -ne 0 +PERF_EXIT_CODE=$? + +# check that the error message is reasonable +../common/check_all_patterns_found.pl "Failed to find" "somenonexistingrandomstuffwhichisalsoprettylongorevenlongertoexceed64" < $LOGS_DIR/adding_kernel_nonexisting.err +CHECK_EXIT_CODE=$? +../common/check_all_patterns_found.pl "in this function|at this address" "Error" "Failed to add events" < $LOGS_DIR/adding_kernel_nonexisting.err +(( CHECK_EXIT_CODE += $? )) +../common/check_all_lines_matched.pl "Failed to find" "Error" "Probe point .+ not found" "optimized out" "Use.+\-\-range option to show.+location range" < $LOGS_DIR/adding_kernel_nonexisting.err +(( CHECK_EXIT_CODE += $? )) +../common/check_no_patterns_found.pl "$RE_SEGFAULT" < $LOGS_DIR/adding_kernel_nonexisting.err +(( CHECK_EXIT_CODE += $? )) + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "non-existing variable" +(( TEST_RESULT += $? )) + + +### function with return value + +# adding probe with return value +$CMD_PERF probe --add "$TEST_PROBE%return \$retval" 2> $LOGS_DIR/adding_kernel_func_retval_add.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "Added new events?:" "probe:$TEST_PROBE" "on $TEST_PROBE%return with \\\$retval" < $LOGS_DIR/adding_kernel_func_retval_add.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "function with retval :: add" +(( TEST_RESULT += $? )) + +# recording some data +$CMD_PERF record -e probe:$TEST_PROBE\* -o $CURRENT_TEST_DIR/perf.data -- cat /proc/cpuinfo > /dev/null 2> $LOGS_DIR/adding_kernel_func_retval_record.err +PERF_EXIT_CODE=$? + +../common/check_all_patterns_found.pl "$RE_LINE_RECORD1" "$RE_LINE_RECORD2" < $LOGS_DIR/adding_kernel_func_retval_record.err +CHECK_EXIT_CODE=$? + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "function with retval :: record" +(( TEST_RESULT += $? )) + +# perf script should report the function calls with the correct arg values +$CMD_PERF script -i $CURRENT_TEST_DIR/perf.data > $LOGS_DIR/adding_kernel_func_retval_script.log +PERF_EXIT_CODE=$? + +REGEX_SCRIPT_LINE="\s*cat\s+$RE_NUMBER\s+\[$RE_NUMBER\]\s+$RE_NUMBER:\s+probe:$TEST_PROBE\w*:\s+\($RE_NUMBER_HEX\s+<\-\s+$RE_NUMBER_HEX\)\s+arg1=$RE_NUMBER_HEX" +../common/check_all_lines_matched.pl "$REGEX_SCRIPT_LINE" < $LOGS_DIR/adding_kernel_func_retval_script.log +CHECK_EXIT_CODE=$? +../common/check_all_patterns_found.pl "$REGEX_SCRIPT_LINE" < $LOGS_DIR/adding_kernel_func_retval_script.log +(( CHECK_EXIT_CODE += $? )) + +print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "function argument probing :: script" +(( TEST_RESULT += $? )) + + +clear_all_probes + +# print overall results +print_overall_results "$TEST_RESULT" +exit $? -- cgit v1.2.3 From 8b767db3309595a23eff1c3f2498f17b1f3a9bbc Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Sat, 17 Feb 2024 15:40:42 +0800 Subject: perf: build: introduce the libcapstone Later we will use libcapstone to disassemble instructions of samples. Signed-off-by: Changbin Du Reviewed-by: Adrian Hunter Cc: changbin.du@gmail.com Cc: Thomas Richter Cc: Andi Kleen Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240217074046.4100789-2-changbin.du@huawei.com --- tools/build/Makefile.feature | 2 ++ tools/build/feature/Makefile | 4 ++++ tools/build/feature/test-all.c | 4 ++++ tools/build/feature/test-libcapstone.c | 11 +++++++++++ tools/perf/Makefile.config | 21 +++++++++++++++++++++ tools/perf/Makefile.perf | 3 +++ tools/perf/builtin-version.c | 1 + tools/perf/tests/make | 4 +++- 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tools/build/feature/test-libcapstone.c (limited to 'tools/perf/tests') diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature index 64df118376df..1e2ab148d5db 100644 --- a/tools/build/Makefile.feature +++ b/tools/build/Makefile.feature @@ -87,6 +87,7 @@ FEATURE_TESTS_EXTRA := \ gtk2-infobar \ hello \ libbabeltrace \ + libcapstone \ libbfd-liberty \ libbfd-liberty-z \ libopencsd \ @@ -134,6 +135,7 @@ FEATURE_DISPLAY ?= \ libcrypto \ libunwind \ libdw-dwarf-unwind \ + libcapstone \ zlib \ lzma \ get_cpuid \ diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 37722e509eb9..ed54cef450f5 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -54,6 +54,7 @@ FILES= \ test-timerfd.bin \ test-libdw-dwarf-unwind.bin \ test-libbabeltrace.bin \ + test-libcapstone.bin \ test-compile-32.bin \ test-compile-x32.bin \ test-zlib.bin \ @@ -286,6 +287,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin: $(OUTPUT)test-libbabeltrace.bin: $(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace) +$(OUTPUT)test-libcapstone.bin: + $(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone) + $(OUTPUT)test-compile-32.bin: $(CC) -m32 -o $@ test-compile.c diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c index 6f4bf386a3b5..dd0a18c2ef8f 100644 --- a/tools/build/feature/test-all.c +++ b/tools/build/feature/test-all.c @@ -134,6 +134,10 @@ #undef main #endif +#define main main_test_libcapstone +# include "test-libcapstone.c" +#undef main + #define main main_test_lzma # include "test-lzma.c" #undef main diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c new file mode 100644 index 000000000000..fbe8dba189e9 --- /dev/null +++ b/tools/build/feature/test-libcapstone.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include + +int main(void) +{ + csh handle; + + cs_open(CS_ARCH_X86, CS_MODE_64, &handle); + return 0; +} diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 51059e45c122..f054f6959b24 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -166,6 +166,15 @@ endif FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS) FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf +# for linking with debug library, run like: +# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/ +ifdef LIBCAPSTONE_DIR + LIBCAPSTONE_CFLAGS := -I$(LIBCAPSTONE_DIR)/include + LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/ +endif +FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS) +FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone + ifdef LIBZSTD_DIR LIBZSTD_CFLAGS := -I$(LIBZSTD_DIR)/lib LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib @@ -1075,6 +1084,18 @@ ifndef NO_LIBBABELTRACE endif endif +ifndef NO_CAPSTONE + $(call feature_check,libcapstone) + ifeq ($(feature-libcapstone), 1) + CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS) + LDFLAGS += $(LICAPSTONE_LDFLAGS) + EXTLIBS += -lcapstone + $(call detected,CONFIG_LIBCAPSTONE) + else + msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel); + endif +endif + ifndef NO_AUXTRACE ifeq ($(SRCARCH),x86) ifeq ($(feature-get_cpuid), 0) diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 3cecd51b2397..86afdaad246f 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -84,6 +84,9 @@ include ../scripts/utilities.mak # Define NO_LIBBABELTRACE if you do not want libbabeltrace support # for CTF data format. # +# Define NO_CAPSTONE if you do not want libcapstone support +# for disasm engine. +# # Define NO_LZMA if you do not want to support compressed (xz) kernel modules # # Define NO_AUXTRACE if you do not want AUX area tracing support diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c index 529e9ce8c46c..1bafe5855ae7 100644 --- a/tools/perf/builtin-version.c +++ b/tools/perf/builtin-version.c @@ -73,6 +73,7 @@ static void library_status(void) STATUS(HAVE_LIBCRYPTO_SUPPORT, libcrypto); STATUS(HAVE_LIBUNWIND_SUPPORT, libunwind); STATUS(HAVE_DWARF_SUPPORT, libdw-dwarf-unwind); + STATUS(HAVE_LIBCAPSTONE_SUPPORT, libcapstone); STATUS(HAVE_ZLIB_SUPPORT, zlib); STATUS(HAVE_LZMA_SUPPORT, lzma); STATUS(HAVE_AUXTRACE_SUPPORT, get_cpuid); diff --git a/tools/perf/tests/make b/tools/perf/tests/make index 8a4da7eb637a..a1f8adf85367 100644 --- a/tools/perf/tests/make +++ b/tools/perf/tests/make @@ -83,6 +83,7 @@ make_no_libelf := NO_LIBELF=1 make_no_libunwind := NO_LIBUNWIND=1 make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1 make_no_backtrace := NO_BACKTRACE=1 +make_no_libcapstone := NO_CAPSTONE=1 make_no_libnuma := NO_LIBNUMA=1 make_no_libaudit := NO_LIBAUDIT=1 make_no_libbionic := NO_LIBBIONIC=1 @@ -122,7 +123,7 @@ make_minimal += NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1 make_minimal += NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1 make_minimal += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1 make_minimal += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1 -make_minimal += NO_LIBCAP=1 NO_SYSCALL_TABLE=1 +make_minimal += NO_LIBCAP=1 NO_SYSCALL_TABLE=1 NO_CAPSTONE=1 # $(run) contains all available tests run := make_pure @@ -152,6 +153,7 @@ run += make_no_libelf run += make_no_libunwind run += make_no_libdw_dwarf_unwind run += make_no_backtrace +run += make_no_libcapstone run += make_no_libnuma run += make_no_libaudit run += make_no_libbionic -- cgit v1.2.3 From 526f2ac9f6a1d668fddf925897b55341bef22644 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 20 Feb 2024 19:41:50 -0800 Subject: perf tests: Avoid fork in perf_has_symbol test perf test -vv Symbols is used to indentify symbols within the perf binary. Add the -F flag so that the test command doesn't fork the test before running. This removes a little overhead. Acked-by: Adrian Hunter Signed-off-by: Ian Rogers Cc: James Clark Cc: Justin Stitt Cc: Bill Wendling Cc: Nick Desaulniers Cc: Yang Jihong Cc: Nathan Chancellor Cc: Kan Liang Cc: Athira Jajeev Cc: llvm@lists.linux.dev Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240221034155.1500118-4-irogers@google.com --- tools/perf/tests/shell/lib/perf_has_symbol.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/shell/lib/perf_has_symbol.sh b/tools/perf/tests/shell/lib/perf_has_symbol.sh index 5d59c32ae3e7..561c93b75d77 100644 --- a/tools/perf/tests/shell/lib/perf_has_symbol.sh +++ b/tools/perf/tests/shell/lib/perf_has_symbol.sh @@ -3,7 +3,7 @@ perf_has_symbol() { - if perf test -vv "Symbols" 2>&1 | grep "[[:space:]]$1$"; then + if perf test -vv -F "Symbols" 2>&1 | grep "[[:space:]]$1$"; then echo "perf does have symbol '$1'" return 0 fi -- cgit v1.2.3 From d5bcade989a86caa4314aa91d6d3f652e8a82fe5 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 20 Feb 2024 19:41:52 -0800 Subject: perf test: Rename builtin-test-list and add missed header guard builtin-test-list is primarily concerned with shell script tests. Rename the file to better reflect this and add a missed header guard. Signed-off-by: Ian Rogers Cc: James Clark Cc: Justin Stitt Cc: Bill Wendling Cc: Nick Desaulniers Cc: Yang Jihong Cc: Nathan Chancellor Cc: Kan Liang Cc: Athira Jajeev Cc: llvm@lists.linux.dev Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240221034155.1500118-6-irogers@google.com --- tools/perf/tests/Build | 2 +- tools/perf/tests/builtin-test-list.c | 207 ----------------------------------- tools/perf/tests/builtin-test-list.h | 12 -- tools/perf/tests/builtin-test.c | 2 +- tools/perf/tests/tests-scripts.c | 207 +++++++++++++++++++++++++++++++++++ tools/perf/tests/tests-scripts.h | 16 +++ 6 files changed, 225 insertions(+), 221 deletions(-) delete mode 100644 tools/perf/tests/builtin-test-list.c delete mode 100644 tools/perf/tests/builtin-test-list.h create mode 100644 tools/perf/tests/tests-scripts.c create mode 100644 tools/perf/tests/tests-scripts.h (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index 53ba9c3e20e0..c7f9d9676095 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 perf-y += builtin-test.o -perf-y += builtin-test-list.o +perf-y += tests-scripts.o perf-y += parse-events.o perf-y += dso-data.o perf-y += attr.o diff --git a/tools/perf/tests/builtin-test-list.c b/tools/perf/tests/builtin-test-list.c deleted file mode 100644 index a65b9e547d82..000000000000 --- a/tools/perf/tests/builtin-test-list.c +++ /dev/null @@ -1,207 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "builtin.h" -#include "builtin-test-list.h" -#include "color.h" -#include "debug.h" -#include "hist.h" -#include "intlist.h" -#include "string2.h" -#include "symbol.h" -#include "tests.h" -#include "util/rlimit.h" - - -/* - * As this is a singleton built once for the run of the process, there is - * no value in trying to free it and just let it stay around until process - * exits when it's cleaned up. - */ -static size_t files_num = 0; -static struct script_file *files = NULL; -static int files_max_width = 0; - -static const char *shell_tests__dir(char *path, size_t size) -{ - const char *devel_dirs[] = { "./tools/perf/tests", "./tests", }; - char *exec_path; - unsigned int i; - - for (i = 0; i < ARRAY_SIZE(devel_dirs); ++i) { - struct stat st; - - if (!lstat(devel_dirs[i], &st)) { - scnprintf(path, size, "%s/shell", devel_dirs[i]); - if (!lstat(devel_dirs[i], &st)) - return path; - } - } - - /* Then installed path. */ - exec_path = get_argv_exec_path(); - scnprintf(path, size, "%s/tests/shell", exec_path); - free(exec_path); - return path; -} - -static const char *shell_test__description(char *description, size_t size, - const char *path, const char *name) -{ - FILE *fp; - char filename[PATH_MAX]; - int ch; - - path__join(filename, sizeof(filename), path, name); - fp = fopen(filename, "r"); - if (!fp) - return NULL; - - /* Skip first line - should be #!/bin/sh Shebang */ - do { - ch = fgetc(fp); - } while (ch != EOF && ch != '\n'); - - description = fgets(description, size, fp); - fclose(fp); - - /* Assume first char on line is omment everything after that desc */ - return description ? strim(description + 1) : NULL; -} - -/* Is this full file path a shell script */ -static bool is_shell_script(const char *path) -{ - const char *ext; - - ext = strrchr(path, '.'); - if (!ext) - return false; - if (!strcmp(ext, ".sh")) { /* Has .sh extension */ - if (access(path, R_OK | X_OK) == 0) /* Is executable */ - return true; - } - return false; -} - -/* Is this file in this dir a shell script (for test purposes) */ -static bool is_test_script(const char *path, const char *name) -{ - char filename[PATH_MAX]; - - path__join(filename, sizeof(filename), path, name); - if (!is_shell_script(filename)) return false; - return true; -} - -/* Duplicate a string and fall over and die if we run out of memory */ -static char *strdup_check(const char *str) -{ - char *newstr; - - newstr = strdup(str); - if (!newstr) { - pr_err("Out of memory while duplicating test script string\n"); - abort(); - } - return newstr; -} - -static void append_script(const char *dir, const char *file, const char *desc) -{ - struct script_file *files_tmp; - size_t files_num_tmp; - int width; - - files_num_tmp = files_num + 1; - if (files_num_tmp >= SIZE_MAX) { - pr_err("Too many script files\n"); - abort(); - } - /* Realloc is good enough, though we could realloc by chunks, not that - * anyone will ever measure performance here */ - files_tmp = realloc(files, - (files_num_tmp + 1) * sizeof(struct script_file)); - if (files_tmp == NULL) { - pr_err("Out of memory while building test list\n"); - abort(); - } - /* Add file to end and NULL terminate the struct array */ - files = files_tmp; - files_num = files_num_tmp; - files[files_num - 1].dir = strdup_check(dir); - files[files_num - 1].file = strdup_check(file); - files[files_num - 1].desc = strdup_check(desc); - files[files_num].dir = NULL; - files[files_num].file = NULL; - files[files_num].desc = NULL; - - width = strlen(desc); /* Track max width of desc */ - if (width > files_max_width) - files_max_width = width; -} - -static void append_scripts_in_dir(const char *path) -{ - struct dirent **entlist; - struct dirent *ent; - int n_dirs, i; - char filename[PATH_MAX]; - - /* List files, sorted by alpha */ - n_dirs = scandir(path, &entlist, NULL, alphasort); - if (n_dirs == -1) - return; - for (i = 0; i < n_dirs && (ent = entlist[i]); i++) { - if (ent->d_name[0] == '.') - continue; /* Skip hidden files */ - if (is_test_script(path, ent->d_name)) { /* It's a test */ - char bf[256]; - const char *desc = shell_test__description - (bf, sizeof(bf), path, ent->d_name); - - if (desc) /* It has a desc line - valid script */ - append_script(path, ent->d_name, desc); - } else if (is_directory(path, ent)) { /* Scan the subdir */ - path__join(filename, sizeof(filename), - path, ent->d_name); - append_scripts_in_dir(filename); - } - } - for (i = 0; i < n_dirs; i++) /* Clean up */ - zfree(&entlist[i]); - free(entlist); -} - -const struct script_file *list_script_files(void) -{ - char path_dir[PATH_MAX]; - const char *path; - - if (files) - return files; /* Singleton - we already know our list */ - - path = shell_tests__dir(path_dir, sizeof(path_dir)); /* Walk dir */ - append_scripts_in_dir(path); - - return files; -} - -int list_script_max_width(void) -{ - list_script_files(); /* Ensure we have scanned all scripts */ - return files_max_width; -} diff --git a/tools/perf/tests/builtin-test-list.h b/tools/perf/tests/builtin-test-list.h deleted file mode 100644 index eb81f3aa6683..000000000000 --- a/tools/perf/tests/builtin-test-list.h +++ /dev/null @@ -1,12 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ - -struct script_file { - char *dir; - char *file; - char *desc; -}; - -/* List available script tests to run - singleton - never freed */ -const struct script_file *list_script_files(void); -/* Get maximum width of description string */ -int list_script_max_width(void); diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 4a5973f9bb9b..eff3c62e9b47 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -29,7 +29,7 @@ #include #include -#include "builtin-test-list.h" +#include "tests-scripts.h" static bool dont_fork; const char *dso_to_test; diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c new file mode 100644 index 000000000000..4ebd841da05b --- /dev/null +++ b/tools/perf/tests/tests-scripts.c @@ -0,0 +1,207 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "builtin.h" +#include "tests-scripts.h" +#include "color.h" +#include "debug.h" +#include "hist.h" +#include "intlist.h" +#include "string2.h" +#include "symbol.h" +#include "tests.h" +#include "util/rlimit.h" + + +/* + * As this is a singleton built once for the run of the process, there is + * no value in trying to free it and just let it stay around until process + * exits when it's cleaned up. + */ +static size_t files_num = 0; +static struct script_file *files = NULL; +static int files_max_width = 0; + +static const char *shell_tests__dir(char *path, size_t size) +{ + const char *devel_dirs[] = { "./tools/perf/tests", "./tests", }; + char *exec_path; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(devel_dirs); ++i) { + struct stat st; + + if (!lstat(devel_dirs[i], &st)) { + scnprintf(path, size, "%s/shell", devel_dirs[i]); + if (!lstat(devel_dirs[i], &st)) + return path; + } + } + + /* Then installed path. */ + exec_path = get_argv_exec_path(); + scnprintf(path, size, "%s/tests/shell", exec_path); + free(exec_path); + return path; +} + +static const char *shell_test__description(char *description, size_t size, + const char *path, const char *name) +{ + FILE *fp; + char filename[PATH_MAX]; + int ch; + + path__join(filename, sizeof(filename), path, name); + fp = fopen(filename, "r"); + if (!fp) + return NULL; + + /* Skip first line - should be #!/bin/sh Shebang */ + do { + ch = fgetc(fp); + } while (ch != EOF && ch != '\n'); + + description = fgets(description, size, fp); + fclose(fp); + + /* Assume first char on line is omment everything after that desc */ + return description ? strim(description + 1) : NULL; +} + +/* Is this full file path a shell script */ +static bool is_shell_script(const char *path) +{ + const char *ext; + + ext = strrchr(path, '.'); + if (!ext) + return false; + if (!strcmp(ext, ".sh")) { /* Has .sh extension */ + if (access(path, R_OK | X_OK) == 0) /* Is executable */ + return true; + } + return false; +} + +/* Is this file in this dir a shell script (for test purposes) */ +static bool is_test_script(const char *path, const char *name) +{ + char filename[PATH_MAX]; + + path__join(filename, sizeof(filename), path, name); + if (!is_shell_script(filename)) return false; + return true; +} + +/* Duplicate a string and fall over and die if we run out of memory */ +static char *strdup_check(const char *str) +{ + char *newstr; + + newstr = strdup(str); + if (!newstr) { + pr_err("Out of memory while duplicating test script string\n"); + abort(); + } + return newstr; +} + +static void append_script(const char *dir, const char *file, const char *desc) +{ + struct script_file *files_tmp; + size_t files_num_tmp; + int width; + + files_num_tmp = files_num + 1; + if (files_num_tmp >= SIZE_MAX) { + pr_err("Too many script files\n"); + abort(); + } + /* Realloc is good enough, though we could realloc by chunks, not that + * anyone will ever measure performance here */ + files_tmp = realloc(files, + (files_num_tmp + 1) * sizeof(struct script_file)); + if (files_tmp == NULL) { + pr_err("Out of memory while building test list\n"); + abort(); + } + /* Add file to end and NULL terminate the struct array */ + files = files_tmp; + files_num = files_num_tmp; + files[files_num - 1].dir = strdup_check(dir); + files[files_num - 1].file = strdup_check(file); + files[files_num - 1].desc = strdup_check(desc); + files[files_num].dir = NULL; + files[files_num].file = NULL; + files[files_num].desc = NULL; + + width = strlen(desc); /* Track max width of desc */ + if (width > files_max_width) + files_max_width = width; +} + +static void append_scripts_in_dir(const char *path) +{ + struct dirent **entlist; + struct dirent *ent; + int n_dirs, i; + char filename[PATH_MAX]; + + /* List files, sorted by alpha */ + n_dirs = scandir(path, &entlist, NULL, alphasort); + if (n_dirs == -1) + return; + for (i = 0; i < n_dirs && (ent = entlist[i]); i++) { + if (ent->d_name[0] == '.') + continue; /* Skip hidden files */ + if (is_test_script(path, ent->d_name)) { /* It's a test */ + char bf[256]; + const char *desc = shell_test__description + (bf, sizeof(bf), path, ent->d_name); + + if (desc) /* It has a desc line - valid script */ + append_script(path, ent->d_name, desc); + } else if (is_directory(path, ent)) { /* Scan the subdir */ + path__join(filename, sizeof(filename), + path, ent->d_name); + append_scripts_in_dir(filename); + } + } + for (i = 0; i < n_dirs; i++) /* Clean up */ + zfree(&entlist[i]); + free(entlist); +} + +const struct script_file *list_script_files(void) +{ + char path_dir[PATH_MAX]; + const char *path; + + if (files) + return files; /* Singleton - we already know our list */ + + path = shell_tests__dir(path_dir, sizeof(path_dir)); /* Walk dir */ + append_scripts_in_dir(path); + + return files; +} + +int list_script_max_width(void) +{ + list_script_files(); /* Ensure we have scanned all scripts */ + return files_max_width; +} diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h new file mode 100644 index 000000000000..3a3ec6191848 --- /dev/null +++ b/tools/perf/tests/tests-scripts.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef TESTS_SCRIPTS_H +#define TESTS_SCRIPTS_H + +struct script_file { + char *dir; + char *file; + char *desc; +}; + +/* List available script tests to run - singleton - never freed */ +const struct script_file *list_script_files(void); +/* Get maximum width of description string */ +int list_script_max_width(void); + +#endif /* TESTS_SCRIPTS_H */ -- cgit v1.2.3 From f3295f5b067d3c2655f0b2cd14d0b91b83ca41eb Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 20 Feb 2024 19:41:53 -0800 Subject: perf tests: Use scandirat for shell script finding Avoid filename appending buffers by using openat, faccessat and scandirat more widely. Turn the script's path back to a file name using readlink from /proc//fd/. Read the script's description using api/io.h to avoid fdopen conversions. Whilst reading perform additional sanity checks on the script's contents. Signed-off-by: Ian Rogers Cc: James Clark Cc: Justin Stitt Cc: Bill Wendling Cc: Nick Desaulniers Cc: Yang Jihong Cc: Nathan Chancellor Cc: Kan Liang Cc: Athira Jajeev Cc: llvm@lists.linux.dev Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240221034155.1500118-7-irogers@google.com --- tools/perf/tests/builtin-test.c | 20 +++--- tools/perf/tests/tests-scripts.c | 145 +++++++++++++++++++++++---------------- tools/perf/tests/tests-scripts.h | 1 - 3 files changed, 95 insertions(+), 71 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index eff3c62e9b47..162f9eb090ac 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -300,22 +300,19 @@ static int test_and_print(struct test_suite *t, int subtest) } struct shell_test { - const char *dir; const char *file; }; static int shell_test__run(struct test_suite *test, int subdir __maybe_unused) { int err; - char script[PATH_MAX]; struct shell_test *st = test->priv; + char *cmd = NULL; - path__join(script, sizeof(script) - 3, st->dir, st->file); - - if (verbose > 0) - strncat(script, " -v", sizeof(script) - strlen(script) - 1); - - err = system(script); + if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0) + return TEST_FAIL; + err = system(cmd); + free(cmd); if (!err) return TEST_OK; @@ -331,7 +328,7 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width, files = list_script_files(); if (!files) return 0; - for (file = files; file->dir; file++) { + for (file = files; file->file; file++) { int curr = i++; struct test_case test_cases[] = { { @@ -345,13 +342,12 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width, .test_cases = test_cases, .priv = &st, }; - st.dir = file->dir; + st.file = file->file; if (test_suite.desc == NULL || !perf_test__matches(test_suite.desc, curr, argc, argv)) continue; - st.file = file->file; pr_info("%3d: %-*s:", i, width, test_suite.desc); if (intlist__find(skiplist, i)) { @@ -455,7 +451,7 @@ static int perf_test__list_shell(int argc, const char **argv, int i) files = list_script_files(); if (!files) return 0; - for (file = files; file->dir; file++) { + for (file = files; file->file; file++) { int curr = i++; struct test_suite t = { .desc = file->desc diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c index 4ebd841da05b..c21f7a425da9 100644 --- a/tools/perf/tests/tests-scripts.c +++ b/tools/perf/tests/tests-scripts.c @@ -14,6 +14,7 @@ #include #include #include +#include #include "builtin.h" #include "tests-scripts.h" #include "color.h" @@ -24,6 +25,7 @@ #include "symbol.h" #include "tests.h" #include "util/rlimit.h" +#include "util/util.h" /* @@ -35,55 +37,69 @@ static size_t files_num = 0; static struct script_file *files = NULL; static int files_max_width = 0; -static const char *shell_tests__dir(char *path, size_t size) +static int shell_tests__dir_fd(void) { - const char *devel_dirs[] = { "./tools/perf/tests", "./tests", }; - char *exec_path; - unsigned int i; + char path[PATH_MAX], *exec_path; + static const char * const devel_dirs[] = { "./tools/perf/tests/shell", "./tests/shell", }; - for (i = 0; i < ARRAY_SIZE(devel_dirs); ++i) { - struct stat st; + for (size_t i = 0; i < ARRAY_SIZE(devel_dirs); ++i) { + int fd = open(devel_dirs[i], O_PATH); - if (!lstat(devel_dirs[i], &st)) { - scnprintf(path, size, "%s/shell", devel_dirs[i]); - if (!lstat(devel_dirs[i], &st)) - return path; - } + if (fd >= 0) + return fd; } /* Then installed path. */ exec_path = get_argv_exec_path(); - scnprintf(path, size, "%s/tests/shell", exec_path); + scnprintf(path, sizeof(path), "%s/tests/shell", exec_path); free(exec_path); - return path; + return open(path, O_PATH); } -static const char *shell_test__description(char *description, size_t size, - const char *path, const char *name) +static char *shell_test__description(int dir_fd, const char *name) { - FILE *fp; - char filename[PATH_MAX]; - int ch; + struct io io; + char buf[128], desc[256]; + int ch, pos = 0; - path__join(filename, sizeof(filename), path, name); - fp = fopen(filename, "r"); - if (!fp) + io__init(&io, openat(dir_fd, name, O_RDONLY), buf, sizeof(buf)); + if (io.fd < 0) return NULL; /* Skip first line - should be #!/bin/sh Shebang */ + if (io__get_char(&io) != '#') + goto err_out; + if (io__get_char(&io) != '!') + goto err_out; do { - ch = fgetc(fp); - } while (ch != EOF && ch != '\n'); - - description = fgets(description, size, fp); - fclose(fp); + ch = io__get_char(&io); + if (ch < 0) + goto err_out; + } while (ch != '\n'); - /* Assume first char on line is omment everything after that desc */ - return description ? strim(description + 1) : NULL; + do { + ch = io__get_char(&io); + if (ch < 0) + goto err_out; + } while (ch == '#' || isspace(ch)); + while (ch > 0 && ch != '\n') { + desc[pos++] = ch; + if (pos >= (int)sizeof(desc) - 1) + break; + ch = io__get_char(&io); + } + while (pos > 0 && isspace(desc[--pos])) + ; + desc[++pos] = '\0'; + close(io.fd); + return strdup(desc); +err_out: + close(io.fd); + return NULL; } /* Is this full file path a shell script */ -static bool is_shell_script(const char *path) +static bool is_shell_script(int dir_fd, const char *path) { const char *ext; @@ -91,20 +107,16 @@ static bool is_shell_script(const char *path) if (!ext) return false; if (!strcmp(ext, ".sh")) { /* Has .sh extension */ - if (access(path, R_OK | X_OK) == 0) /* Is executable */ + if (faccessat(dir_fd, path, R_OK | X_OK, 0) == 0) /* Is executable */ return true; } return false; } /* Is this file in this dir a shell script (for test purposes) */ -static bool is_test_script(const char *path, const char *name) +static bool is_test_script(int dir_fd, const char *name) { - char filename[PATH_MAX]; - - path__join(filename, sizeof(filename), path, name); - if (!is_shell_script(filename)) return false; - return true; + return is_shell_script(dir_fd, name); } /* Duplicate a string and fall over and die if we run out of memory */ @@ -120,12 +132,21 @@ static char *strdup_check(const char *str) return newstr; } -static void append_script(const char *dir, const char *file, const char *desc) +static void append_script(int dir_fd, const char *name, char *desc) { + char filename[PATH_MAX], link[128]; struct script_file *files_tmp; - size_t files_num_tmp; + size_t files_num_tmp, len; int width; + snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd); + len = readlink(link, filename, sizeof(filename)); + if (len < 0) { + pr_err("Failed to readlink %s", link); + return; + } + filename[len++] = '/'; + strcpy(&filename[len], name); files_num_tmp = files_num + 1; if (files_num_tmp >= SIZE_MAX) { pr_err("Too many script files\n"); @@ -142,10 +163,8 @@ static void append_script(const char *dir, const char *file, const char *desc) /* Add file to end and NULL terminate the struct array */ files = files_tmp; files_num = files_num_tmp; - files[files_num - 1].dir = strdup_check(dir); - files[files_num - 1].file = strdup_check(file); - files[files_num - 1].desc = strdup_check(desc); - files[files_num].dir = NULL; + files[files_num - 1].file = strdup_check(filename); + files[files_num - 1].desc = desc; files[files_num].file = NULL; files[files_num].desc = NULL; @@ -154,32 +173,39 @@ static void append_script(const char *dir, const char *file, const char *desc) files_max_width = width; } -static void append_scripts_in_dir(const char *path) +static void append_scripts_in_dir(int dir_fd) { struct dirent **entlist; struct dirent *ent; int n_dirs, i; - char filename[PATH_MAX]; /* List files, sorted by alpha */ - n_dirs = scandir(path, &entlist, NULL, alphasort); + n_dirs = scandirat(dir_fd, ".", &entlist, NULL, alphasort); if (n_dirs == -1) return; for (i = 0; i < n_dirs && (ent = entlist[i]); i++) { + int fd; + if (ent->d_name[0] == '.') continue; /* Skip hidden files */ - if (is_test_script(path, ent->d_name)) { /* It's a test */ - char bf[256]; - const char *desc = shell_test__description - (bf, sizeof(bf), path, ent->d_name); + if (is_test_script(dir_fd, ent->d_name)) { /* It's a test */ + char *desc = shell_test__description(dir_fd, ent->d_name); if (desc) /* It has a desc line - valid script */ - append_script(path, ent->d_name, desc); - } else if (is_directory(path, ent)) { /* Scan the subdir */ - path__join(filename, sizeof(filename), - path, ent->d_name); - append_scripts_in_dir(filename); + append_script(dir_fd, ent->d_name, desc); + continue; + } + if (ent->d_type != DT_DIR) { + struct stat st; + + if (ent->d_type != DT_UNKNOWN) + continue; + fstatat(dir_fd, ent->d_name, &st, 0); + if (!S_ISDIR(st.st_mode)) + continue; } + fd = openat(dir_fd, ent->d_name, O_PATH); + append_scripts_in_dir(fd); } for (i = 0; i < n_dirs; i++) /* Clean up */ zfree(&entlist[i]); @@ -188,14 +214,17 @@ static void append_scripts_in_dir(const char *path) const struct script_file *list_script_files(void) { - char path_dir[PATH_MAX]; - const char *path; + int dir_fd; if (files) return files; /* Singleton - we already know our list */ - path = shell_tests__dir(path_dir, sizeof(path_dir)); /* Walk dir */ - append_scripts_in_dir(path); + dir_fd = shell_tests__dir_fd(); /* Walk dir */ + if (dir_fd < 0) + return NULL; + + append_scripts_in_dir(dir_fd); + close(dir_fd); return files; } diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h index 3a3ec6191848..3508a293aaf9 100644 --- a/tools/perf/tests/tests-scripts.h +++ b/tools/perf/tests/tests-scripts.h @@ -3,7 +3,6 @@ #define TESTS_SCRIPTS_H struct script_file { - char *dir; char *file; char *desc; }; -- cgit v1.2.3 From 964461ee370f3c0d63c173bfe4e4995f66d91578 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 20 Feb 2024 19:41:54 -0800 Subject: perf tests: Run time generate shell test suites Rather than special shell test logic, do a single pass to create an array of test suites. Hold the shell test file name in the test suite priv field. This makes the special shell test logic in builtin-test.c redundant so remove it. Signed-off-by: Ian Rogers Cc: James Clark Cc: Justin Stitt Cc: Bill Wendling Cc: Nick Desaulniers Cc: Yang Jihong Cc: Nathan Chancellor Cc: Kan Liang Cc: Athira Jajeev Cc: llvm@lists.linux.dev Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240221034155.1500118-8-irogers@google.com --- tools/perf/tests/builtin-test.c | 90 +-------------------------- tools/perf/tests/tests-scripts.c | 129 +++++++++++++++++++++++---------------- tools/perf/tests/tests-scripts.h | 10 +-- 3 files changed, 80 insertions(+), 149 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 162f9eb090ac..c42cb40fc242 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -130,6 +130,7 @@ static struct test_suite *generic_tests[] = { static struct test_suite **tests[] = { generic_tests, arch_tests, + NULL, /* shell tests created at runtime. */ }; static struct test_workload *workloads[] = { @@ -299,73 +300,12 @@ static int test_and_print(struct test_suite *t, int subtest) return err; } -struct shell_test { - const char *file; -}; - -static int shell_test__run(struct test_suite *test, int subdir __maybe_unused) -{ - int err; - struct shell_test *st = test->priv; - char *cmd = NULL; - - if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0) - return TEST_FAIL; - err = system(cmd); - free(cmd); - if (!err) - return TEST_OK; - - return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL; -} - -static int run_shell_tests(int argc, const char *argv[], int i, int width, - struct intlist *skiplist) -{ - struct shell_test st; - const struct script_file *files, *file; - - files = list_script_files(); - if (!files) - return 0; - for (file = files; file->file; file++) { - int curr = i++; - struct test_case test_cases[] = { - { - .desc = file->desc, - .run_case = shell_test__run, - }, - { .name = NULL, } - }; - struct test_suite test_suite = { - .desc = test_cases[0].desc, - .test_cases = test_cases, - .priv = &st, - }; - st.file = file->file; - - if (test_suite.desc == NULL || - !perf_test__matches(test_suite.desc, curr, argc, argv)) - continue; - - pr_info("%3d: %-*s:", i, width, test_suite.desc); - - if (intlist__find(skiplist, i)) { - color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n"); - continue; - } - - test_and_print(&test_suite, 0); - } - return 0; -} - static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) { struct test_suite *t; unsigned int j, k; int i = 0; - int width = list_script_max_width(); + int width = 0; for_each_test(j, k, t) { int len = strlen(test_description(t, -1)); @@ -440,28 +380,6 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) } } } - - return run_shell_tests(argc, argv, i, width, skiplist); -} - -static int perf_test__list_shell(int argc, const char **argv, int i) -{ - const struct script_file *files, *file; - - files = list_script_files(); - if (!files) - return 0; - for (file = files; file->file; file++) { - int curr = i++; - struct test_suite t = { - .desc = file->desc - }; - - if (!perf_test__matches(t.desc, curr, argc, argv)) - continue; - - pr_info("%3d: %s\n", i, t.desc); - } return 0; } @@ -488,9 +406,6 @@ static int perf_test__list(int argc, const char **argv) test_description(t, subi)); } } - - perf_test__list_shell(argc, argv, i); - return 0; } @@ -550,6 +465,7 @@ int cmd_test(int argc, const char **argv) /* Unbuffered output */ setvbuf(stdout, NULL, _IONBF, 0); + tests[2] = create_script_test_suites(); argc = parse_options_subcommand(argc, argv, test_options, test_subcommands, test_usage, 0); if (argc >= 1 && !strcmp(argv[0], "list")) return perf_test__list(argc - 1, argv + 1); diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c index c21f7a425da9..e2042b368269 100644 --- a/tools/perf/tests/tests-scripts.c +++ b/tools/perf/tests/tests-scripts.c @@ -27,16 +27,6 @@ #include "util/rlimit.h" #include "util/util.h" - -/* - * As this is a singleton built once for the run of the process, there is - * no value in trying to free it and just let it stay around until process - * exits when it's cleaned up. - */ -static size_t files_num = 0; -static struct script_file *files = NULL; -static int files_max_width = 0; - static int shell_tests__dir_fd(void) { char path[PATH_MAX], *exec_path; @@ -132,12 +122,30 @@ static char *strdup_check(const char *str) return newstr; } -static void append_script(int dir_fd, const char *name, char *desc) +static int shell_test__run(struct test_suite *test, int subtest __maybe_unused) +{ + const char *file = test->priv; + int err; + char *cmd = NULL; + + if (asprintf(&cmd, "%s%s", file, verbose ? " -v" : "") < 0) + return TEST_FAIL; + err = system(cmd); + free(cmd); + if (!err) + return TEST_OK; + + return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL; +} + +static void append_script(int dir_fd, const char *name, char *desc, + struct test_suite ***result, + size_t *result_sz) { char filename[PATH_MAX], link[128]; - struct script_file *files_tmp; - size_t files_num_tmp, len; - int width; + struct test_suite *test_suite, **result_tmp; + struct test_case *tests; + size_t len; snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd); len = readlink(link, filename, sizeof(filename)); @@ -147,33 +155,43 @@ static void append_script(int dir_fd, const char *name, char *desc) } filename[len++] = '/'; strcpy(&filename[len], name); - files_num_tmp = files_num + 1; - if (files_num_tmp >= SIZE_MAX) { - pr_err("Too many script files\n"); - abort(); + + tests = calloc(2, sizeof(*tests)); + if (!tests) { + pr_err("Out of memory while building script test suite list\n"); + return; } + tests[0].name = strdup_check(name); + tests[0].desc = strdup_check(desc); + tests[0].run_case = shell_test__run; + + test_suite = zalloc(sizeof(*test_suite)); + if (!test_suite) { + pr_err("Out of memory while building script test suite list\n"); + free(tests); + return; + } + test_suite->desc = desc; + test_suite->test_cases = tests; + test_suite->priv = strdup_check(filename); /* Realloc is good enough, though we could realloc by chunks, not that * anyone will ever measure performance here */ - files_tmp = realloc(files, - (files_num_tmp + 1) * sizeof(struct script_file)); - if (files_tmp == NULL) { - pr_err("Out of memory while building test list\n"); - abort(); + result_tmp = realloc(*result, (*result_sz + 1) * sizeof(*result_tmp)); + if (result_tmp == NULL) { + pr_err("Out of memory while building script test suite list\n"); + free(tests); + free(test_suite); + return; } /* Add file to end and NULL terminate the struct array */ - files = files_tmp; - files_num = files_num_tmp; - files[files_num - 1].file = strdup_check(filename); - files[files_num - 1].desc = desc; - files[files_num].file = NULL; - files[files_num].desc = NULL; - - width = strlen(desc); /* Track max width of desc */ - if (width > files_max_width) - files_max_width = width; + *result = result_tmp; + (*result)[*result_sz] = test_suite; + (*result_sz)++; } -static void append_scripts_in_dir(int dir_fd) +static void append_scripts_in_dir(int dir_fd, + struct test_suite ***result, + size_t *result_sz) { struct dirent **entlist; struct dirent *ent; @@ -192,7 +210,7 @@ static void append_scripts_in_dir(int dir_fd) char *desc = shell_test__description(dir_fd, ent->d_name); if (desc) /* It has a desc line - valid script */ - append_script(dir_fd, ent->d_name, desc); + append_script(dir_fd, ent->d_name, desc, result, result_sz); continue; } if (ent->d_type != DT_DIR) { @@ -205,32 +223,35 @@ static void append_scripts_in_dir(int dir_fd) continue; } fd = openat(dir_fd, ent->d_name, O_PATH); - append_scripts_in_dir(fd); + append_scripts_in_dir(fd, result, result_sz); } for (i = 0; i < n_dirs; i++) /* Clean up */ zfree(&entlist[i]); free(entlist); } -const struct script_file *list_script_files(void) +struct test_suite **create_script_test_suites(void) { - int dir_fd; - - if (files) - return files; /* Singleton - we already know our list */ - - dir_fd = shell_tests__dir_fd(); /* Walk dir */ - if (dir_fd < 0) - return NULL; + struct test_suite **result = NULL, **result_tmp; + size_t result_sz = 0; + int dir_fd = shell_tests__dir_fd(); /* Walk dir */ - append_scripts_in_dir(dir_fd); - close(dir_fd); + /* + * Append scripts if fd is good, otherwise return a NULL terminated zero + * length array. + */ + if (dir_fd >= 0) + append_scripts_in_dir(dir_fd, &result, &result_sz); - return files; -} - -int list_script_max_width(void) -{ - list_script_files(); /* Ensure we have scanned all scripts */ - return files_max_width; + result_tmp = realloc(result, (result_sz + 1) * sizeof(*result_tmp)); + if (result_tmp == NULL) { + pr_err("Out of memory while building script test suite list\n"); + abort(); + } + /* NULL terminate the test suite array. */ + result = result_tmp; + result[result_sz] = NULL; + if (dir_fd >= 0) + close(dir_fd); + return result; } diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h index 3508a293aaf9..b553ad26ea17 100644 --- a/tools/perf/tests/tests-scripts.h +++ b/tools/perf/tests/tests-scripts.h @@ -2,14 +2,8 @@ #ifndef TESTS_SCRIPTS_H #define TESTS_SCRIPTS_H -struct script_file { - char *file; - char *desc; -}; +#include "tests.h" -/* List available script tests to run - singleton - never freed */ -const struct script_file *list_script_files(void); -/* Get maximum width of description string */ -int list_script_max_width(void); +struct test_suite **create_script_test_suites(void); #endif /* TESTS_SCRIPTS_H */ -- cgit v1.2.3 From b482f5f8e0168f1e81bbb45c5238a3bed481818a Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 20 Feb 2024 19:41:55 -0800 Subject: perf tests: Add option to run tests in parallel By default tests are forked, add an option (-p or --parallel) so that the forked tests are all started in parallel and then their output gathered serially. This is opt-in as running in parallel can cause test flakes. Rather than fork within the code, the start_command/finish_command from libsubcmd are used. This changes how stderr and stdout are handled. The child stderr and stdout are always read to avoid the child blocking. If verbose is 1 (-v) then if the test fails the child stdout and stderr are displayed. If the verbose is >1 (e.g. -vv) then the stdout and stderr from the child are immediately displayed. An unscientific test on my laptop shows the wall clock time for perf test without parallel being 5 minutes 21 seconds and with parallel (-p) being 1 minute 50 seconds. Signed-off-by: Ian Rogers Cc: James Clark Cc: Justin Stitt Cc: Bill Wendling Cc: Nick Desaulniers Cc: Yang Jihong Cc: Nathan Chancellor Cc: Kan Liang Cc: Athira Jajeev Cc: llvm@lists.linux.dev Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240221034155.1500118-9-irogers@google.com --- tools/perf/tests/builtin-test.c | 314 +++++++++++++++++++++++++++------------- 1 file changed, 215 insertions(+), 99 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index c42cb40fc242..d13ee7683d9d 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -6,6 +6,7 @@ */ #include #include +#include #include #include #include @@ -21,9 +22,11 @@ #include "debug.h" #include "color.h" #include +#include #include "string2.h" #include "symbol.h" #include "util/rlimit.h" +#include "util/strbuf.h" #include #include #include @@ -31,7 +34,13 @@ #include "tests-scripts.h" +/* + * Command line option to not fork the test running in the same process and + * making them easier to debug. + */ static bool dont_fork; +/* Fork the tests in parallel and then wait for their completion. */ +static bool parallel; const char *dso_to_test; const char *test_objdump_path = "objdump"; @@ -209,76 +218,36 @@ static bool perf_test__matches(const char *desc, int curr, int argc, const char return false; } -static int run_test(struct test_suite *test, int subtest) -{ - int status, err = -1, child = dont_fork ? 0 : fork(); - char sbuf[STRERR_BUFSIZE]; - - if (child < 0) { - pr_err("failed to fork test: %s\n", - str_error_r(errno, sbuf, sizeof(sbuf))); - return -1; - } - - if (!child) { - if (!dont_fork) { - pr_debug("test child forked, pid %d\n", getpid()); - - if (verbose <= 0) { - int nullfd = open("/dev/null", O_WRONLY); - - if (nullfd >= 0) { - close(STDERR_FILENO); - close(STDOUT_FILENO); - - dup2(nullfd, STDOUT_FILENO); - dup2(STDOUT_FILENO, STDERR_FILENO); - close(nullfd); - } - } else { - signal(SIGSEGV, sighandler_dump_stack); - signal(SIGFPE, sighandler_dump_stack); - } - } - - err = test_function(test, subtest)(test, subtest); - if (!dont_fork) - exit(err); - } - - if (!dont_fork) { - wait(&status); +struct child_test { + struct child_process process; + struct test_suite *test; + int test_num; + int subtest; +}; - if (WIFEXITED(status)) { - err = (signed char)WEXITSTATUS(status); - pr_debug("test child finished with %d\n", err); - } else if (WIFSIGNALED(status)) { - err = -1; - pr_debug("test child interrupted\n"); - } - } +static int run_test_child(struct child_process *process) +{ + struct child_test *child = container_of(process, struct child_test, process); + int err; - return err; + pr_debug("--- start ---\n"); + pr_debug("test child forked, pid %d\n", getpid()); + err = test_function(child->test, child->subtest)(child->test, child->subtest); + pr_debug("---- end(%d) ----\n", err); + fflush(NULL); + return -err; } -#define for_each_test(j, k, t) \ - for (j = 0, k = 0; j < ARRAY_SIZE(tests); j++, k = 0) \ - while ((t = tests[j][k++]) != NULL) - -static int test_and_print(struct test_suite *t, int subtest) +static int print_test_result(struct test_suite *t, int i, int subtest, int result, int width) { - int err; + if (has_subtests(t)) { + int subw = width > 2 ? width - 2 : width; - pr_debug("\n--- start ---\n"); - err = run_test(t, subtest); - pr_debug("---- end ----\n"); + pr_info("%3d.%1d: %-*s:", i + 1, subtest + 1, subw, test_description(t, subtest)); + } else + pr_info("%3d: %-*s:", i + 1, width, test_description(t, subtest)); - if (!has_subtests(t)) - pr_debug("%s:", t->desc); - else - pr_debug("%s subtest %d:", t->desc, subtest + 1); - - switch (err) { + switch (result) { case TEST_OK: pr_info(" Ok\n"); break; @@ -297,34 +266,186 @@ static int test_and_print(struct test_suite *t, int subtest) break; } - return err; + return 0; +} + +static int finish_test(struct child_test *child_test, int width) +{ + struct test_suite *t = child_test->test; + int i = child_test->test_num; + int subi = child_test->subtest; + int out = child_test->process.out; + int err = child_test->process.err; + bool out_done = out <= 0; + bool err_done = err <= 0; + struct strbuf out_output = STRBUF_INIT; + struct strbuf err_output = STRBUF_INIT; + int ret; + + /* + * For test suites with subtests, display the suite name ahead of the + * sub test names. + */ + if (has_subtests(t) && subi == 0) + pr_info("%3d: %-*s:\n", i + 1, width, test_description(t, -1)); + + /* + * Busy loop reading from the child's stdout and stderr that are set to + * be non-blocking until EOF. + */ + if (!out_done) + fcntl(out, F_SETFL, O_NONBLOCK); + if (!err_done) + fcntl(err, F_SETFL, O_NONBLOCK); + if (verbose > 1) { + if (has_subtests(t)) + pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi)); + else + pr_info("%3d: %s:\n", i + 1, test_description(t, -1)); + } + while (!out_done || !err_done) { + struct pollfd pfds[2] = { + { .fd = out, + .events = POLLIN | POLLERR | POLLHUP | POLLNVAL, + }, + { .fd = err, + .events = POLLIN | POLLERR | POLLHUP | POLLNVAL, + }, + }; + char buf[512]; + ssize_t len; + + /* Poll to avoid excessive spinning, timeout set for 1000ms. */ + poll(pfds, ARRAY_SIZE(pfds), /*timeout=*/1000); + if (!out_done && pfds[0].revents) { + errno = 0; + len = read(out, buf, sizeof(buf) - 1); + + if (len <= 0) { + out_done = errno != EAGAIN; + } else { + buf[len] = '\0'; + if (verbose > 1) + fprintf(stdout, "%s", buf); + else + strbuf_addstr(&out_output, buf); + } + } + if (!err_done && pfds[1].revents) { + errno = 0; + len = read(err, buf, sizeof(buf) - 1); + + if (len <= 0) { + err_done = errno != EAGAIN; + } else { + buf[len] = '\0'; + if (verbose > 1) + fprintf(stdout, "%s", buf); + else + strbuf_addstr(&err_output, buf); + } + } + } + /* Clean up child process. */ + ret = finish_command(&child_test->process); + if (verbose == 1 && ret == TEST_FAIL) { + /* Add header for test that was skipped above. */ + if (has_subtests(t)) + pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi)); + else + pr_info("%3d: %s:\n", i + 1, test_description(t, -1)); + fprintf(stdout, "%s", out_output.buf); + fprintf(stderr, "%s", err_output.buf); + } + strbuf_release(&out_output); + strbuf_release(&err_output); + print_test_result(t, i, subi, ret, width); + if (out > 0) + close(out); + if (err > 0) + close(err); + return 0; +} + +static int start_test(struct test_suite *test, int i, int subi, struct child_test **child, + int width) +{ + int err; + + *child = NULL; + if (dont_fork) { + pr_debug("--- start ---\n"); + err = test_function(test, subi)(test, subi); + pr_debug("---- end ----\n"); + print_test_result(test, i, subi, err, width); + return 0; + } + + *child = zalloc(sizeof(**child)); + if (!*child) + return -ENOMEM; + + (*child)->test = test; + (*child)->test_num = i; + (*child)->subtest = subi; + (*child)->process.pid = -1; + (*child)->process.no_stdin = 1; + if (verbose <= 0) { + (*child)->process.no_stdout = 1; + (*child)->process.no_stderr = 1; + } else { + (*child)->process.out = -1; + (*child)->process.err = -1; + } + (*child)->process.no_exec_cmd = run_test_child; + err = start_command(&(*child)->process); + if (err || parallel) + return err; + return finish_test(*child, width); } +#define for_each_test(j, k, t) \ + for (j = 0, k = 0; j < ARRAY_SIZE(tests); j++, k = 0) \ + while ((t = tests[j][k++]) != NULL) + static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) { struct test_suite *t; unsigned int j, k; int i = 0; int width = 0; + size_t num_tests = 0; + struct child_test **child_tests; + int child_test_num = 0; for_each_test(j, k, t) { int len = strlen(test_description(t, -1)); if (width < len) width = len; + + if (has_subtests(t)) { + for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) { + len = strlen(test_description(t, subi)); + if (width < len) + width = len; + num_tests++; + } + } else { + num_tests++; + } } + child_tests = calloc(num_tests, sizeof(*child_tests)); + if (!child_tests) + return -ENOMEM; for_each_test(j, k, t) { int curr = i++; - int subi; if (!perf_test__matches(test_description(t, -1), curr, argc, argv)) { bool skip = true; - int subn; - - subn = num_subtests(t); - for (subi = 0; subi < subn; subi++) { + for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) { if (perf_test__matches(test_description(t, subi), curr, argc, argv)) skip = false; @@ -334,52 +455,45 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) continue; } - pr_info("%3d: %-*s:", i, width, test_description(t, -1)); - if (intlist__find(skiplist, i)) { + pr_info("%3d: %-*s:", curr + 1, width, test_description(t, -1)); color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n"); continue; } if (!has_subtests(t)) { - test_and_print(t, -1); - } else { - int subn = num_subtests(t); - /* - * minus 2 to align with normal testcases. - * For subtest we print additional '.x' in number. - * for example: - * - * 35: Test LLVM searching and compiling : - * 35.1: Basic BPF llvm compiling test : Ok - */ - int subw = width > 2 ? width - 2 : width; - - if (subn <= 0) { - color_fprintf(stderr, PERF_COLOR_YELLOW, - " Skip (not compiled in)\n"); - continue; - } - pr_info("\n"); + int err = start_test(t, curr, -1, &child_tests[child_test_num++], width); - for (subi = 0; subi < subn; subi++) { - int len = strlen(test_description(t, subi)); - - if (subw < len) - subw = len; + if (err) { + /* TODO: if parallel waitpid the already forked children. */ + free(child_tests); + return err; } + } else { + for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) { + int err; - for (subi = 0; subi < subn; subi++) { if (!perf_test__matches(test_description(t, subi), curr, argc, argv)) continue; - pr_info("%3d.%1d: %-*s:", i, subi + 1, subw, - test_description(t, subi)); - test_and_print(t, subi); + err = start_test(t, curr, subi, &child_tests[child_test_num++], + width); + if (err) + return err; } } } + for (i = 0; i < child_test_num; i++) { + if (parallel) { + int ret = finish_test(child_tests[i], width); + + if (ret) + return ret; + } + free(child_tests[i]); + } + free(child_tests); return 0; } @@ -447,6 +561,8 @@ int cmd_test(int argc, const char **argv) "be more verbose (show symbol address, etc)"), OPT_BOOLEAN('F', "dont-fork", &dont_fork, "Do not fork for testcase"), + OPT_BOOLEAN('p', "parallel", ¶llel, + "Run the tests altogether in parallel"), OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"), OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"), OPT_STRING(0, "objdump", &test_objdump_path, "path", -- cgit v1.2.3 From 8680999dbe5735a68feae396dcfc486e18679f2e Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 22 Feb 2024 17:07:20 -0300 Subject: perf test: Use TEST_FAIL in the TEST_ASSERT macros instead of -1 Just to make things clearer, return TEST_FAIL (-1) instead of an open coded -1. Signed-off-by: Arnaldo Carvalho de Melo Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/ZdepeMsjagbf1ufD@x1 --- tools/perf/tests/tests.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index dad3d7414142..3aa7701ee0e9 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -4,11 +4,17 @@ #include +enum { + TEST_OK = 0, + TEST_FAIL = -1, + TEST_SKIP = -2, +}; + #define TEST_ASSERT_VAL(text, cond) \ do { \ if (!(cond)) { \ pr_debug("FAILED %s:%d %s\n", __FILE__, __LINE__, text); \ - return -1; \ + return TEST_FAIL; \ } \ } while (0) @@ -17,16 +23,10 @@ do { \ if (val != expected) { \ pr_debug("FAILED %s:%d %s (%d != %d)\n", \ __FILE__, __LINE__, text, val, expected); \ - return -1; \ + return TEST_FAIL; \ } \ } while (0) -enum { - TEST_OK = 0, - TEST_FAIL = -1, - TEST_SKIP = -2, -}; - struct test_suite; typedef int (*test_fnptr)(struct test_suite *, int); -- cgit v1.2.3 From eb94225eb469a163280a6cf939fe0e5ea708fd29 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 26 Feb 2024 10:53:26 +0000 Subject: perf test: Fix spelling mistake "curent" -> "current" There is a spelling mistake in a pr_debug message. Fix it. Signed-off-by: Colin Ian King Reviewed-by: Adrian Hunter Cc: kernel-janitors@vger.kernel.org Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240226105326.3944887-1-colin.i.king@gmail.com --- tools/perf/tests/symbols.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/tests') diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c index 2fed6d67f50f..d208105919ed 100644 --- a/tools/perf/tests/symbols.c +++ b/tools/perf/tests/symbols.c @@ -85,7 +85,7 @@ static int create_map(struct test_info *ti, char *filename, struct map **map_p) *map_p = find_module_map(ti->machine, dso); dso__put(dso); if (!*map_p) { - pr_debug("Failed to find map for curent kernel module %s", + pr_debug("Failed to find map for current kernel module %s", filename); return TEST_FAIL; } -- cgit v1.2.3