top(1) loses process user time count when threads end

[John Baldwin]

On Saturday, August 06, 2011 1:57:35 pm Yuri wrote:
> On 08/06/2011 02:11, Alexander Best wrote:
> > On Fri Aug  5 11, Yuri wrote:
> >> I have the process that first runs in 3 threads but later two active
> >> threads exit.
> >>
> >> top(1) shows this moment this way (1 sec intervals):
> >> 30833 yuri            3  76    0  4729M  4225M nanslp  4   0:32 88.62% app
> >> 30833 yuri            3  76    0  4729M  4225M nanslp  6   0:34 90.92% app
> >> 30833 yuri            1  96    0  4729M  4225M CPU1    1   0:03  1.17% app
> >> 30833 yuri            1  98    0  4729M  4226M CPU1    1   0:04 12.89% app
> >>
> >> Process time goes down: 0:34 ->  0:03. Also WCPU goes down 90.92% ->
> >> 1.17% even though this process is CPU bound and does intense things
> >> right after threads exit.
> >>
> >> getrusage(2) though, called in the process, shows the correct user time.
> >>
> >> I think this is the major bug in the process time accounting.
> > could you check, whether kern/128177 or kern/140892 describe your situation?
> 
> I have ULE scheduler. kern/128177 talks about single thread with ULE 
> scheduler, and my issue is with threads. So I am not sure if it is 
> related. There have been no motion on kern/128177 since Feb 9, 2009.
> kern/140892 is probably the same as mine.
> 
> In any case, both these PRs have to be fixed since they are very user 
> visible, not just some obscure issues.

You can try this perhaps:

Index: kern/kern_thread.c
===================================================================
--- kern/kern_thread.c	(revision 224879)
+++ kern/kern_thread.c	(working copy)
@@ -381,7 +381,7 @@
 void
 thread_exit(void)
 {
-	uint64_t new_switchtime;
+	uint64_t runtime, new_switchtime;
 	struct thread *td;
 	struct thread *td2;
 	struct proc *p;
@@ -410,15 +410,6 @@
 	 */
 	cpu_thread_exit(td);	/* XXXSMP */
 
-	/* Do the same timestamp bookkeeping that mi_switch() would do. */
-	new_switchtime = cpu_ticks();
-	p->p_rux.rux_runtime += (new_switchtime - PCPU_GET(switchtime));
-	PCPU_SET(switchtime, new_switchtime);
-	PCPU_SET(switchticks, ticks);
-	PCPU_INC(cnt.v_swtch);
-	/* Save our resource usage in our process. */
-	td->td_ru.ru_nvcsw++;
-	rucollect(&p->p_ru, &td->td_ru);
 	/*
 	 * The last thread is left attached to the process
 	 * So that the whole bundle gets recycled. Skip
@@ -467,7 +458,21 @@
 		PMC_SWITCH_CONTEXT(td, PMC_FN_CSW_OUT);
 #endif
 	PROC_UNLOCK(p);
+
+	/* Do the same timestamp bookkeeping that mi_switch() would do. */
+	new_switchtime = cpu_ticks();
+	runtime = new_switchtime - PCPU_GET(switchtime);
+	td->td_runtime += runtime;
+	td->td_incruntime += runtime;
+	PCPU_SET(switchtime, new_switchtime);
+	PCPU_SET(switchticks, ticks);
+	PCPU_INC(cnt.v_swtch);
+
+	/* Save our resource usage in our process. */
+	td->td_ru.ru_nvcsw++;
 	ruxagg(p, td);
+	rucollect(&p->p_ru, &td->td_ru);
+
 	thread_lock(td);
 	PROC_SUNLOCK(p);
 	td->td_state = TDS_INACTIVE;

-- 
John Baldwin