| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes a bug where a binary linked using max-page-size=0x200000
can result in a bogus relocation offset when running on a system
with a smaller page size. This causes samples to fall outside
the image mapping or be translated to the wrong address (resulting
in symbol resolution, or incorrect symbol resolution). We noticed
this at Netflix because we run a patchset enabling 16k pages on
amd64 and have been compiling userspace with a 2MB page size.
Since we started doing this profiling userspace binaries has been
wonky.
Reviewed by: ali_mashtizadeh.com
Differential Revision: https://reviews.freebsd.org/D59771
Sponsored by: Netflix
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remove struct pmchdr_cpuidinfo which was just a wrapper around a
flexible array member of uint32_t. Flexible array members are
non-standard in C++, and even in C are not allowed as the only member
of a struct.
GCC errored out on pmchdr_cpuidinfo, but did not complain about
pmchdr_pmcinfo, so I left it alone here, though it is also non-standard.
Fixes: 93da997ef759 ("pmc: new pmc log processing framework")
Reviewed by: Ali Mashtizadeh <ali@mashtizadeh.com>
Differential Revision: https://reviews.freebsd.org/D59355
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The previous check did not work on architectures where `char` is
unsigned as noted by GCC on aarch64:
usr.sbin/pmc/view.cc: In member function 'void pmcview::loadsymboltable(image*, Elf*, Elf_Scn*, GElf_Shdr*)':
usr.sbin/pmc/view.cc:627:38: error: comparison is always false due to limited range of data type [-Werror=type-limits]
627 | if (fname[i] < 0) {
| ~~~~~~~~~^~~
Reported by: GCC 15
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This pacifies shadow warnings from GCC:
usr.sbin/pmc/cmd_pmc_record.cc: In constructor 'pmc_config::pmc_config(const std::__1::string&, uint64_t, cpuset_t)':
usr.sbin/pmc/cmd_pmc_record.cc:96:71: error: declaration of 'cpumask' shadows a member of 'pmc_config' [-Werror=shadow]
96 | pmc_config(const std::string &event, uint64_t count, cpuset_t cpumask)
| ~~~~~~~~~^~~~~~~
usr.sbin/pmc/cmd_pmc_record.cc:90:25: note: shadowed declaration is here
90 | cpuset_t cpumask;
| ^~~~~~~
usr.sbin/pmc/cmd_pmc_record.cc:96:55: error: declaration of 'count' shadows a member of 'pmc_config' [-Werror=shadow]
96 | pmc_config(const std::string &event, uint64_t count, cpuset_t cpumask)
| ~~~~~~~~~^~~~~
usr.sbin/pmc/cmd_pmc_record.cc:89:25: note: shadowed declaration is here
89 | uint64_t count;
| ^~~~~
usr.sbin/pmc/cmd_pmc_record.cc:96:39: error: declaration of 'event' shadows a member of 'pmc_config' [-Werror=shadow]
96 | pmc_config(const std::string &event, uint64_t count, cpuset_t cpumask)
| ~~~~~~~~~~~~~~~~~~~^~~~~
usr.sbin/pmc/cmd_pmc_record.cc:88:25: note: shadowed declaration is here
88 | std::string event;
| ^~~~~
Reported by: GCC 15
Fixes: a79a051e7d16 ("pmc: pmc record command")
|
| |
|
|
|
|
|
|
|
| |
I need to do more work before references can be accepted in other
sections of the code. This was an unnecessary drive-by change that was
not tested in `make universe`.
Reported by: CI
Fixes: fd809148 ("pmc(8): resolve -Wshadow issues")
|
| |
|
|
|
|
|
| |
This mutes a number of complains from g++ about needing specific headers
for functionality related to C strings and other function prototypes.
Reported by: g++ 14
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
- Prefix all structs with the struct keyword to avoid collisions between
the types and variables with the same "name".
- Use `_` suffixed variables in initializers to distinguish input
parameters from public members [1].
Resolve some trailing whitespace issues while here.
NOTE: this doesn't resolve the -pedantic issue reported by g++ with
`pmchdr_cpuidinfo::cpuid` about the field being a flexible array in an
otherwise empty struct.
1. I generally do this the other way around, i.e., suffix private/protected
members with `_`, but these are public members in structs and I don't want
to introduce a lot of churn in calling code.
Reported by: g++14 with FreeBSD CI (powerpc64 tinderbox)
Fixes: ce6ab51f ("pmc: enable the new pmc commands")
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This change hooks everything up to the pmc command and improves the
usage to document all functions. There are a couple older commands that
are currently broken that I have hidden from the usage, but left in the
code for those using it. I won't remove those until we have our
replacements upstreamed that depend on the AMD PMC multiplexing patches.
Sponsored by: Netflix
Reviewed by: adrian, imp
Differential Revision: https://reviews.freebsd.org/D57780
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The frontend command uses AMD IBS frontend events to analyze the major
sources of frontend stalls. It displays a table breakind down the major
causes of front end stalls. This is a simple demonstration of the tools
as you can use the filtering tools to limit the analysis to a subset of
the samples including filtering by fetch latencies.
Sponsored by: Netflix
Reviewed by: adrian, imp
Differential Revision: https://reviews.freebsd.org/D57779
|
| |
|
|
|
|
|
|
|
| |
Prints the log header including machine, cpu and kernel details along
with what counters were selected.
Sponsored by: Netflix
Reviewed by: adrian, imp
Differential Revision: https://reviews.freebsd.org/D57778
|
| |
|
|
|
|
|
|
|
|
|
| |
The record command is designed around the idea of predefined studies.
While you can still select individual counters, the predefined studies
are meant to enable the best hardware options for a given generation.
It implements all of the base studies that I have built so far.
Sponsored by: Netflix
Reviewed by: adrian
Differential Revision: https://reviews.freebsd.org/D57777
|
| |
|
|
|
|
|
|
|
|
|
| |
View is a class for building PMC log processing tools it is designed to
work with the new PMC record command that adds a header with additional
CPU information. The new framework processes PMC logs about 2.5 times
faster and in about half the code as libpmcstat.
Sponsored by: Netflix
Reviewed by: adrian
Differential Revision: https://reviews.freebsd.org/D57776
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Initializes the terminal rendering code used by the new pmc tools. Then
provides a table abstraction for collecting, sorting and rendering
tables. It provides pretty printed results with typed fields that print
several types used throughout the new PMC tools. By default the fields
are formatted in engineering notation.
Sponsored by: Netflix
Reviewed by: adrian, imp
Differential Revision: https://reviews.freebsd.org/D57775
|
| |
|
|
|
|
|
|
|
| |
Due to the size of libpmc.so.5, this reduces the size of the -utilities
package by 10%.
MFC after: 1 day
Reviewed by: manu, adrian, emaste
Differential Revision: https://reviews.freebsd.org/D52662
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously the compiler's default C++ standard was used unlike C where
bsd.sys.mk explicitly sets a default language version. Setting an
explicit default version will give a more uniform experience across
different compilers and compiler versions.
gnu++17 was chosen to match the default C standard. It is well
supported by a wide range of clang (5+) and GCC (9+) versions.
gnu++17 is also the default C++ standard in recent versions of clang
(16+) and GCC (11+). As a result, many of the explicit CXXSTD
settings in Makefiles had the effect of lowering the C++ standard
instead of raising it as was originally intended and are removed.
Note that the remaining explicit CXXSTD settings for atf and liblutok
explicitly lower the standard to C++11 due to use of the deprecated
auto_ptr<> template which is removed in later versions.
Reviewed by: imp, asomers, dim, emaste
Differential Revision: https://reviews.freebsd.org/D49223
|
| |
|
|
|
|
|
|
|
|
|
|
| |
- For some reason we don't build it as a PIE, but I don't have any
problems doing so with either clang or gcc.
- There is no apparent need to override WARNS, so don't.
- Some building with -O0, presumably that's left over from debugging.
MFC after: 1 week
Reviewed by: imp, brooks
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D43923
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Apply the following automated changes to try to eliminate
no-longer-needed sys/cdefs.h includes as well as now-empty
blank lines in a row.
Remove /^#if.*\n#endif.*\n#include\s+<sys/cdefs.h>.*\n/
Remove /\n+#include\s+<sys/cdefs.h>.*\n+#if.*\n#endif.*\n+/
Remove /\n+#if.*\n#endif.*\n+/
Remove /^#if.*\n#endif.*\n/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/types.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/param.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/capsicum.h>/
Sponsored by: Netflix
|
| |
|
|
| |
Remove /^\s*#[#!]?\s*\$FreeBSD\$.*$\n/
|
| |
|
|
| |
Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/
|
| |
|
|
| |
Remove /^\s*\*\n \*\s+\$FreeBSD\$$\n/
|
| |
|
|
|
|
|
|
|
| |
The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.
Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix
|
| | |
|
| |
|
|
| |
Differential Revision: https://reviews.freebsd.org/D32098
|
| |
|
|
|
|
|
|
|
|
|
| |
There is a single consumer, the pmc utility, that clearly has knowledge
of which counters it is expecting. Remove this function and have it
use common counter aliases instead.
Reviewed by: gnn
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D30528
|
| |
|
|
|
|
|
|
| |
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Notes:
svn path=/head/; revision=368775
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Coverity detected this error. The fix duplicates the assignment on line 171.
Submitted by: bret_ketchum@dell.com
Reported by: Coverity
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D26227
Notes:
svn path=/head/; revision=365051
|
| |
|
|
|
|
|
|
|
| |
PR: 245189
Reported by: Gordon Bergling
Sponsored by: DARPA
Notes:
svn path=/head/; revision=359601
|
| |
|
|
|
|
|
|
| |
Reviewed by: bapt
Differential Revision: https://reviews.freebsd.org/D24173
Notes:
svn path=/head/; revision=359292
|
| |
|
|
|
|
|
|
|
|
|
|
| |
It's invalid to reference a C++ string's c_str() buffer after the object
goes out of scope. Adjust the scope of the string to match the use in
write(2) to fix the misuse.
CID: 1393383
Reported by: Coverity
Notes:
svn path=/head/; revision=348078
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
CXXSTD was added as the C++ analogue to CSTD.
CXXSTD defaults to `-std=c++11` with supporting compilers; `-std=gnu++98`,
otherwise for older versions of g++.
This change standardizes the CXXSTD variable, originally added to
googletest.test.inc.mk as part of r345203.
As part of this effort, convert all `CXXFLAGS+= -std=*` calls to use `CXXSTD`.
Notes:
This value is not sanity checked in bsd.sys.mk, however, given the two
most used C++ compilers on FreeBSD (clang++ and g++) support both modes, it is
likely to work with both toolchains. This method will be refined in the future
to support more variants of C++, as not all versions of clang++ and g++ (for
instance) support C++14, C++17, etc.
Any manual appending of `-std=*` to `CXXFLAGS` should be replaced with CXXSTD.
Example:
Before this commit:
```
CXXFLAGS+= -std=c++14
```
After this commit:
```
CXXSTD= c++14
```
Reviewed by: asomers
Approved by: emaste (mentor)
MFC after: 1 month
MFC with: r345203, r345704, r345705
Relnotes: yes
Tested with: make tinderbox
Differential Revision: https://reviews.freebsd.org/D19732
Notes:
svn path=/head/; revision=345708
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a review is closed via Phabricator it updates the patch attached to the
review. I downloaded the raw patch from Phabricator, applied it, and repeated
my mistake from r345704 by accident mixing content from D19732 and D19738.
For my own personal sanity, I will try not to mix reviews like this in the
future.
MFC after: 1 month
MFC with: r345706
Approved by: emaste (mentor, implicit)
Notes:
svn path=/head/; revision=345707
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
CXXSTD was added as the C++ analogue to CSTD.
CXXSTD defaults to `-std=c++11` with supporting compilers; `-std=gnu++98`,
otherwise for older versions of g++.
This change standardizes the CXXSTD variable, originally added to
googletest.test.inc.mk as part of r345203.
As part of this effort, convert all `CXXFLAGS+= -std=*` calls to use `CXXSTD`.
Notes:
This value is not sanity checked in bsd.sys.mk, however, given the two
most used C++ compilers on FreeBSD (clang++ and g++) support both modes, it is
likely to work with both toolchains. This method will be refined in the future
to support more variants of C++, as not all versions of clang++ and g++ (for
instance) support C++14, C++17, etc.
Any manual appending of `-std=*` to `CXXFLAGS` should be replaced with CXXSTD.
Example:
Before this commit:
```
CXXFLAGS+= -std=c++14
```
After this commit:
```
CXXSTD= c++14
```
Reviewed by: asomers
Approved by: emaste (mentor)
MFC after: 1 month
MFC with: r345203, r345704, r345705
Relnotes: yes
Tested with: make tinderbox
Differential Revision: https://reviews.freebsd.org/D19732
Notes:
svn path=/head/; revision=345706
|
| |
|
|
|
|
|
|
|
|
|
|
| |
I accidentally committed code from two reviews. I will reintroduce the code to
bsd.progs.mk as part of a separate commit from r345704.
Approved by: emaste (mentor, implicit)
MFC after: 2 months
MFC with: r345704
Notes:
svn path=/head/; revision=345705
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
CXXSTD defaults to `-std=c++11` with supporting compilers; `-std=gnu++98`,
otherwise for older versions of g++.
This change standardizes the CXXSTD variable, originally added to
googletest.test.inc.mk as part of r345203.
As part of this effort, convert all `CXXFLAGS+= -std=*` calls to use `CXXSTD`.
Notes:
This value is not sanity checked in bsd.sys.mk, however, given the two
most used C++ compilers on FreeBSD (clang++ and g++) support both modes, it is
likely to work with both toolchains. This method will be refined in the future
to support more variants of C++, as not all versions of clang++ and g++ (for
instance) support C++14, C++17, etc.
Any manual appending of `-std=*` to `CXXFLAGS` should be replaced with CXXSTD.
Example:
Before this commit:
```
CXXFLAGS+= -std=c++14
```
After this commit:
```
CXXSTD= c++14
```
Reviewed by: asomers
Approved by: emaste (mentor)
MFC after: 1 month
Relnotes: yes
Differential Revision: https://reviews.freebsd.org/D19732
Notes:
svn path=/head/; revision=345704
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
GCC 8.1 failed to build LLVM's libc++ when -Wshadow is set,
so lower down WARNS flag to 3.
This is similar to dtc(1) which uses libc++ and sets WARNS to 3.
Approved by: re (gjb)
Sponsored by: DARPA, AFRL
Notes:
svn path=/head/; revision=339064
|
| |
|
|
|
|
|
| |
Reported by: swills
Notes:
svn path=/head/; revision=335045
|
| |
|
|
|
|
|
|
|
|
|
| |
Fix the build of lib/libpmc and usr.sbin/pmc for gcc on amd64.
Reviewed by: mmacy
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D15723
Notes:
svn path=/head/; revision=334957
|
| |
|
|
|
|
|
|
|
| |
Also change a single instance of the comma operator to a semi-colon.
Reviewed by: mmacy
Notes:
svn path=/head/; revision=334874
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
- add '-j' options to filter to enable converting native pmc
log format to json lines format to enable the use of scripts
and external tooling
% pmc filter -j pmc.log pmc.jsonl
- Record the tsc value in sampling interrupts as opposed to
recording nanotime when the sample is copied to a global log
in hardclock - potentially many milliseconds later.
- At initialize record the tsc_freq and the time of day to give
us an offset for translating the tsc values in callchain records
Notes:
svn path=/head/; revision=334749
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
metadata changes:
- log pmc sample rate with pmcallocate
- log proc flags with thread / process logging
to identify user vs kernel threads
fixes:
- use log cpuid to translate event id to event name
Implement rudimentary summary command to track sample
counts by thread and process name within a pmc log.
% make -j4 buildkernel >& /dev/null &
% sudo pmcstat -S unhalted_core_cycles -S llc-misses -O foo sleep 15
% pmc summary foo
cpu_clk_unhalted.thread_p_any:
idle: 138108207162
clang-6.0: 105336158004
sh: 72340108510
make: 8642012963
kernel: 7754011631
longest_lat_cache.miss:
clang-6.0: 87502625
sh: 40901227
make: 5500165
kernel: 3300099
awk: 2000060
% pmc summary -f ~/foo
idx: 278 name: cpu_clk_unhalted.thread_p_any rate: 2000003
idle: 69054
clang-6.0: 52668
sh: 36170
make: 4321
kernel: 3877
hwpmc: proc(7445): 3319
awk: 1289
xargs: 357
rand_harvestq: 181
mtree: 102
intr: 53
zfskern: 31
usb: 7
pagedaemon: 4
ntpd: 3
syslogd: 1
acpi_thermal: 1
logger: 1
syncer: 1
snmptrapd: 1
sleep: 1
idx: 17 name: longest_lat_cache.miss rate: 100003
clang-6.0: 875
sh: 409
make: 55
kernel: 33
awk: 20
hwpmc: proc(7445): 14
xargs: 9
idle: 8
intr: 3
zfskern: 2
Notes:
svn path=/head/; revision=334701
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
By logging all threads and processes 'pmc filter'
can now filter on process or thread name, relieving
the user of the burden of determining which tid or
pid was which when the sample was taken.
% pmc filter -T if_io_tqg -P nginx pmc.log pmc-iflib.log
% pmc filter -x -T idle pmc.log pmc-noidle.log
Notes:
svn path=/head/; revision=334647
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334644
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334641
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334609
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
pmc filter allows the user to select event types, threads, and processes from
a sample.
% pmcstat -S unhalted_core_cycles -S llc-misses -S -S resource_stalls.any -O pmc.log
% pmc filter -e llc-misses pmc.log pmc-llc-misses.log
% pmc filter -e unhalted_core_cycles -t 100339 pmc.log pmc-core-cycles.log
etc...
% pmcstat -R pmc-core-cycles.log -G pmc-core-cycles.stacks
Notes:
svn path=/head/; revision=334601
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334465
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334404
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334361
|
| |
|
|
| |
Notes:
svn path=/head/; revision=334353
|
|
|
This will manage pmc functionality with a more
manageable structure of subcommands rather than the
gradually accreted spaghetti logic of overlapping flags
that exists in pmcstat.
This is intended to ultimately have all the same functionality
as pmcannotate+pmccontrol+pmcstat. Currently it just has
"stat" and "system-stat" - counters for the process itself and counters
for the system as a whole respectively (i.e. system-stat includes kernel
threads). Note that the rusage results (page faults/context switches/
user/sys) for stat-system will not account for the system as a whole -
only for the child process specified on the command line.
Implementing stat was suggested by mjg@ and the output is based on that
from Linux's "perf stat".
% pmc stat -- make -j32 buildkernel -DNO_MODULES -ss > /dev/null
9598393 page faults # 0.674 M/sec
387085 voluntary csw # 0.027 M/sec
106989 involuntary csw # 0.008 M/sec
2763965982317 cycles
2542953049760 instructions # 0.920 inst/cycle
511562750157 branches
12917006881 branch-misses # 2.525%
17944429878 cache-references # 0.007 refs/inst
2205119560 cache-misses # 12.289%
43.74 real # 2019.72% cpu
795.09 user # 1817.72% cpu
88.35 sys # 202.00% cpu
% make -j32 buildkernel -DNO_MODULES -ss > /dev/null &
% sudo pmc stat-system cat
^C 103 page faults # 0.811 M/sec
4 voluntary csw # 0.031 M/sec
0 involuntary csw # 0.000 M/sec
2843639070514 cycles
2606171217438 instructions # 0.916 inst/cycle
522450422783 branches
13092862839 branch-misses # 2.506%
18592101113 cache-references # 0.007 refs/inst
2562878667 cache-misses # 13.785%
44.85 real # 0.00% cpu
0.00 user # 0.00% cpu
0.00 sys # 0.00% cpu
Notes:
svn path=/head/; revision=334350
|