svn commit: r290930 - head/sys/dev/hwpmc

[Jonathan T. Looney]

Author: jtl
Date: Mon Nov 16 15:22:15 2015
New Revision: 290930
URL: https://svnweb.freebsd.org/changeset/base/290930

Log:
  Improve accuracy of PMC sampling frequency
  
  The code tracks a counter which is the number of events until the next
  sample. On context switch in, it loads the saved counter. On context
  switch out, it tries to calculate a new saved counter.
  
  Problems:
  
  1. The saved counter was shared by all threads in a process. However, this
  means that all threads would be initially loaded with the same saved
  counter. However, that could result in sampling more often than once every
  X number of events.
  
  2. The calculation to determine a new saved counter was backwards. It
  added when it should have subtracted, and subtracted when it should have
  added. Assume a single-threaded process with a reload count of 1000 events.
  Assuming the counter on context switch in was 100 and the counter on context
  switch out was 50 (meaning the thread has "consumed" 50 more events), the
  code would calculate a new saved counter of 150 (instead of the proper 50).
  
  Fix:
  
  1. As soon as the saved counter is used to initialize a monitor for a
  thread on context switch in, set the saved counter to the reload count.
  That way, subsequent threads to use the saved counter will get the full
  reload count, assuring we sample at least once every X number of events
  (across all threads).
  
  2. Change the calculation of the saved counter. Due to the change to the
  saved counter in #1, we simply need to add (modulo the reload count) the
  remaining counter time we retrieve from the CPU when a thread is context
  switched out.
  
  Differential Revision:	https://reviews.freebsd.org/D4122
  Approved by:	gnn (mentor)
  MFC after:	1 month
  Sponsored by:	Juniper Networks

Modified:
  head/sys/dev/hwpmc/hwpmc_mod.c

Modified: head/sys/dev/hwpmc/hwpmc_mod.c
==============================================================================
--- head/sys/dev/hwpmc/hwpmc_mod.c	Mon Nov 16 15:16:09 2015	(r290929)
+++ head/sys/dev/hwpmc/hwpmc_mod.c	Mon Nov 16 15:22:15 2015	(r290930)
@@ -1282,8 +1282,16 @@ pmc_process_csw_in(struct thread *td)
 		 */
 		if (PMC_TO_MODE(pm) == PMC_MODE_TS) {
 			mtx_pool_lock_spin(pmc_mtxpool, pm);
+
+			/*
+			 * Use the saved value calculated after the most recent
+			 * thread switch out to start this counter.  Reset
+			 * the saved count in case another thread from this
+			 * process switches in before any threads switch out.
+			 */
 			newvalue = PMC_PCPU_SAVED(cpu,ri) =
 			    pp->pp_pmcs[ri].pp_pmcval;
+			pp->pp_pmcs[ri].pp_pmcval = pm->pm_sc.pm_reloadcount;
 			mtx_pool_unlock_spin(pmc_mtxpool, pm);
 		} else {
 			KASSERT(PMC_TO_MODE(pm) == PMC_MODE_TC,
@@ -1431,31 +1439,43 @@ pmc_process_csw_out(struct thread *td)
 
 			pcd->pcd_read_pmc(cpu, adjri, &newvalue);
 
-			tmp = newvalue - PMC_PCPU_SAVED(cpu,ri);
-
-			PMCDBG3(CSW,SWO,1,"cpu=%d ri=%d tmp=%jd", cpu, ri,
-			    tmp);
-
 			if (mode == PMC_MODE_TS) {
+				PMCDBG3(CSW,SWO,1,"cpu=%d ri=%d tmp=%jd (samp)",
+				    cpu, ri, PMC_PCPU_SAVED(cpu,ri) - newvalue);
 
 				/*
 				 * For sampling process-virtual PMCs,
-				 * we expect the count to be
-				 * decreasing as the 'value'
-				 * programmed into the PMC is the
-				 * number of events to be seen till
-				 * the next sampling interrupt.
+				 * newvalue is the number of events to be seen
+				 * until the next sampling interrupt.
+				 * We can just add the events left from this
+				 * invocation to the counter, then adjust
+				 * in case we overflow our range.
+				 *
+				 * (Recall that we reload the counter every
+				 * time we use it.)
 				 */
-				if (tmp < 0)
-					tmp += pm->pm_sc.pm_reloadcount;
 				mtx_pool_lock_spin(pmc_mtxpool, pm);
-				pp->pp_pmcs[ri].pp_pmcval -= tmp;
-				if ((int64_t) pp->pp_pmcs[ri].pp_pmcval <= 0)
-					pp->pp_pmcs[ri].pp_pmcval +=
+
+				pp->pp_pmcs[ri].pp_pmcval += newvalue;
+				if (pp->pp_pmcs[ri].pp_pmcval >
+				    pm->pm_sc.pm_reloadcount)
+					pp->pp_pmcs[ri].pp_pmcval -=
 					    pm->pm_sc.pm_reloadcount;
+				KASSERT(pp->pp_pmcs[ri].pp_pmcval > 0 &&
+				    pp->pp_pmcs[ri].pp_pmcval <=
+				    pm->pm_sc.pm_reloadcount,
+				    ("[pmc,%d] pp_pmcval outside of expected "
+				    "range cpu=%d ri=%d pp_pmcval=%jx "
+				    "pm_reloadcount=%jx", __LINE__, cpu, ri,
+				    pp->pp_pmcs[ri].pp_pmcval,
+				    pm->pm_sc.pm_reloadcount));
 				mtx_pool_unlock_spin(pmc_mtxpool, pm);
 
 			} else {
+				tmp = newvalue - PMC_PCPU_SAVED(cpu,ri);
+
+				PMCDBG3(CSW,SWO,1,"cpu=%d ri=%d tmp=%jd (count)",
+				    cpu, ri, tmp);
 
 				/*
 				 * For counting process-virtual PMCs,