Re: Updated rusage patch
- From: Jeff Roberson <jroberson@xxxxxxxxxxxxxx>
- Date: Thu, 31 May 2007 18:17:26 -0700 (PDT)
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"
- Follow-Ups:
- Re: Updated rusage patch
- From: Bruce Evans
- Re: Updated rusage patch
- Next by Date: Last chance for threadlock review.
- Next by thread: Re: Updated rusage patch
- Index(es):
Relevant Pages
|
|