Re: Updated rusage patch



On Thu, 31 May 2007, Jeff Roberson wrote:

On Thu, 31 May 2007, Bruce Evans wrote:

I believe the sched lock locking is necessary for rux. For rusage, however, it's only ever modified by curthread. The only races that are present when reading without locks are potentially inconsistent rusage where the stats are not all from a single snapshot of the program. However, the stats can obviously change before the copy makes it back to user-space anyhow. I don't see a real race that needs protecting.


Now that I've said all of that and committed the patch, I just realized that there is still one race that is unacceptable. When the thread exits in thread_exit() and adds the stats of both threads together we could lose changes in the still-running thread.

I guess I may just leave a ru in struct proc that they are added to. In the threadlock patch I serialize thread_exit() on a per-process basis. This will be compatible with the locking there. This will also get rid of that MALLOC you didn't like at the expense of slightly bloating struct proc, which I don't like.

Jeff

This is why I believe the code in exit1() is also safe although technically it could lose some information on the way to thread_exit(). To fix this we'd have to do the final accumulation under sched_lock the last time we lock it. However there are other races in the proc code there that were not immediately obvious. Also, doing the accumulation here negates the possibility of running process accounting after p_ru is valid, which it is not doing now.

Related to rusage but unrelated to my patch; Why are irxss, irdss, and irsss expressed in units of bytes*ticks of execution when rusage doesn't report the ticks of execution and rather a timeval? Are consumers expected to sum the timevals and then extrapolate?

Jeff


Bruce

_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"

_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"



Relevant Pages

  • Re: Updated rusage patch
    ... More notes on locking below. ... the same races as in RELENG_4: ... exit1() uses no locking and thus races with statclock. ... The only races that are present when reading without locks are potentially inconsistent rusage where the stats are not all from a single snapshot of the program. ...
    (freebsd-arch)
  • Re: rusage breakdown and cpu limits.
    ... the rusage from the thread at context switch time). ... This adds our child's rusage to our own. ... the races for the non-times parts of the rusage are lost before the part ...
    (freebsd-arch)