Implement per-thread counters for PMC sampling --- redux
ClosedPublic
Actions

Authored by mmacy on May 7 2018, 5:06 AM.

Details

Reviewers

jhb
jeff
kib
sbruno

Commits

rS333690: hwpmc: Implement per-thread counters for PMC sampling

Summary

This is D4227 with the changes requested by @jhb plus some additional bug fixes exposed on a cursory review. Not entirely sure how to test this until I bring in D7350.

Depends on D15155

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 16617

Event Timeline

mmacy created this revision.May 7 2018, 5:06 AM

Herald added subscribers: gnn, imp. · View Herald TranscriptMay 7 2018, 5:06 AM

mmacy edited the summary of this revision. (Show Details)May 7 2018, 5:16 AM

mmacy added a parent revision: D15155: Make pmclog buffer pcpu and update constants.

mmacy added a reviewer: kib.May 7 2018, 7:09 PM

mmacy added a reviewer: sbruno.May 8 2018, 6:55 AM

kib added inline comments.May 11 2018, 3:43 PM

sys/dev/hwpmc/hwpmc_mod.c
1634	Why is reload subtraction only done for process counter ?
1687	Blank line is excessive.
1701	There too.
2308	Why freelist_mtx needs to be spinlock ?
2315	return (pt);
2352	So if some other consumer drain the pool in parallel, you can end up allocating the entries only to free them. Why not chomp the head of the list of needed length under the freelist_mtx, and then free the entries after unlock ? This also removes lock/unlock for each free.
sys/kern/kern_thr.c
264–268	Why not simply check PMC_PROC_IS_USING_PMCS() after the unlock, without else ? You call pmc hook after the unlock anyway, so it gives the same race.

Apply review suggestions

sys/dev/hwpmc/hwpmc_mod.c
1634	We automatically reset the counter on read. For processes we need to keep track of updates across all threads and then track the delta until the next reload. For threads the value won't exceed reloadcount.
2308	It's called from sched_switch with the thread_lock held.