Page MenuHomeFreeBSD

hwpmc: record page size in PMC logs & fix analysis of binaries linked with large page sizes
ClosedPublic

Authored by gallatin on Thu, Sep 17, 6:45 PM.
Tags
None
Referenced Files
F173143816: D59771.diff
Wed, Sep 23, 10:02 PM
F173092570: D59771.id.diff
Wed, Sep 23, 1:03 PM
Unknown Object (File)
Tue, Sep 22, 8:19 AM
Unknown Object (File)
Tue, Sep 22, 7:41 AM
Unknown Object (File)
Tue, Sep 22, 4:20 AM
Unknown Object (File)
Tue, Sep 22, 1:45 AM
Unknown Object (File)
Mon, Sep 21, 10:24 PM
Unknown Object (File)
Mon, Sep 21, 6:04 PM
Subscribers

Details

Summary

Reuse one byte of the pmc map_in struct to record the capture system’s PAGE_SHIFT, leaving the remaining three bytes reserved and available for future use.

Update pmcstat and pmc to use the recorded page size when calculating user executable mappings. Kernel and kernel-module mappings continue using the ELF segment’s p_align, since their MAP_IN records describe linker load bases.

This fixes a bug where a binary using max-page-size=0x200000 can result in a bogus relocation offset when running on a system with a smaller page size, as this can cause samples to fall outside the image mapping or be translated to the wrong address (resulting in bad symbol resolution). We noticed this because we run a patchset which enables 16K pages on amd64 and we have been compiling our binaries with a 2MB page size. Since we started doing that, profiling userspace binaries has been terrible.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Looks good, I would rather move the field out of the header so we keep those bytes.

FreeBSD/lib/libpmcstat/libpmcstat_image.c
437 ↗(On Diff #186988)

Ah both rtld and the kernel loader round down instead of using the alignment so this seems right.

FreeBSD/sys/sys/pmclog.h
96 ↗(On Diff #186988)

Wish we just added this field to initialize but couldn't we use the padding bytes in pmclog_map_in?

FreeBSD/usr.sbin/pmc/view.cc
737 ↗(On Diff #186988)

I think you want pid == 0 to be safe, there's some issue I can't remember exactly but some parts of pmc bill it -1 and others to 0.

This revision is now accepted and ready to land.Fri, Sep 18, 2:15 AM
gallatin edited the summary of this revision. (Show Details)

Addressed review feedback:

  • Used map_in rather than PMC log header for page shift
  • fixed pid use in view.cc

Tested and confirmed "pmc flamegraph" as well as pmcstat still produce accurate stacks for userspace compiled with larger (2mb) page sizes.

This revision now requires review to proceed.Fri, Sep 18, 7:27 PM
This revision is now accepted and ready to land.Mon, Sep 21, 5:06 PM