Merge pull request #1427 from NLnetLabs/stats-shm-volley-review

stats-shm-volley review with typos and slightly more detailed text for comments.

daemon/daemon.h

@@ -145,7 +145,7 @@ struct daemon {
 #endif
 	/** The SHM info for shared memory stats. */
 	struct shm_main_info* shm_info;
-	/** * If the timeout for statistics is attempted at specific offset.
+	/** if the timeout for statistics is attempted at specific offset.
 	 * If it is true, the stat timeout is the interval+offset, and that
 	 * picks (roughly) the same time offset every time period. */
 	int stat_time_specific;

daemon/worker.c

@@ -2134,6 +2134,9 @@ worker_restart_timer(struct worker* worker)
 			nows = (int)now.tv_sec;
 			/* The next time is on the timer interval, at the
 			 * specific offset, time value % interval = offset. */
+			/* It relies on the integer division below to drop the
+			 * remainder in order to calculate the expected
+			 * result. */
 			spec = ((nows-offset)/interval+1)*interval+offset;
 			/* This is instead of an assertion, and should not
 			 * be needed. So assert(spec > nows), tv is going to

util/shm_side/shm_main.c

@@ -290,22 +290,28 @@ static int
 shm_thread_is_first(struct shm_main_info* shm_info, int thread_num,
 	struct daemon* daemon)
 {
-	/* The usual method, all thread executed last time, and there
-	 * is no statistics in progress. */
+	/* The usual method, all threads executed last time, and there
+	 * is no statistics callback in progress. */
 	if(!shm_info->volley_in_progress)
 		return 1;
-	/* See if we are already active, if so, the timer fired twice
-	 * for this thread during the statistics in progress.
-	 * Another thread is not active during the statistics process,
-	 * so this thread must be the first of this new round without that
-	 * other thread. */
+	/* See if we are already active, if so, the timer seems to have fired
+	 * twice for this thread which means other thread(s) have not gone
+	 * through their stats callbacks yet.
+	 * (There should have been a last thread to reset
+	 * shm_info->volley_in_progress and shm_info->thread_volley)
+	 * The other thread(s) are not yet active during this statistics round,
+	 * so this thread must be the first of this new round disregarding the
+	 * other busy thread(s).
+	 * When the other thread(s) have time again, they will process their
+	 * stats callback and hopefully properly end a stats round where all
+	 * threads got to calculate their statistics. */
 	if(shm_info->thread_volley[thread_num] != 0) {
 		/* The new round starts and zeroes the total. The previous
-		 * partial total is discarded. That means while a thread
-		 * is performing a long task, eg. loading a large zone perhaps,
-		 * the total is not updated and stays the same in the
-		 * shared memory area. Once that thread performs the statistic
-		 * callback again, the total is updated again.
+		 * partial total is discarded. That means while other thread(s)
+		 * are performing a long task, eg. loading a large zone
+		 * perhaps, the total is not updated and stays the same in the
+		 * shared memory area. Once that other thread(s) perform the
+		 * statistic callback again, the total is updated again.
 		 *
 		 * The threads busy with long tasks have 0 in the array.
 		 * The array is inited for a new round. */
@@ -320,8 +326,9 @@ shm_thread_is_first(struct shm_main_info* shm_info, int thread_num,
 static int
 shm_thread_is_last(struct daemon* daemon)
 {
-	/* This means that all threads have been active and this thread
-	 * is the last one. All the thread_volley values are true then. */
+	/* Being last means that all threads have been active for this stats
+	 * round and this thread is the last one; also active. All the
+	 * thread_volley values should be true then. */
 	int i;
 	for(i=0; i<daemon->num; i++) {
 		if(!daemon->shm_info->thread_volley[i])

util/shm_side/shm_main.h

@@ -65,8 +65,10 @@ struct shm_main_info {
 	/** This mutex is on the volley information. */
 	lock_basic_type lock;
 	/** If there is a volley, a number of stat timer callbacks by the
-	 * threads, in progress. If not, it was never started or has ended
-	 * previously. */
+	 * threads, in progress. If not, there is no volley in progress and the
+	 * previous stat run has terminated succesfully for all threads.
+	 * Usually activated by the first thread and deactivated by the last
+	 * thread that starts its stat callback. */
 	int volley_in_progress;
 	/** Per thread, if they have put in stats. 0 if not. */
 	int* thread_volley;