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 Not Applicable

Unit

Tests Not Applicable

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
1626 ↗	(On Diff #42222)	Why is reload subtraction only done for process counter ?
1680 ↗	(On Diff #42222)	Blank line is excessive.
1694 ↗	(On Diff #42222)	There too.
2297 ↗	(On Diff #42222)	Why freelist_mtx needs to be spinlock ?
2304 ↗	(On Diff #42222)	return (pt);
2341 ↗	(On Diff #42222)	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
270 ↗	(On Diff #42222)	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
1626 ↗	(On Diff #42222)	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.
2297 ↗	(On Diff #42222)	It's called from sched_switch with the thread_lock held.