thread0: Clear td_rux stats in proc0_post
ClosedPublic
Actions

Authored by jhb on Tue, Dec 2, 10:44 PM.

Details

Reviewers

markj
kib
olce

Commits

rGcdad10ea780f: thread0: Clear td_rux stats in proc0_post
rGebb0e512d97e: thread0: Clear td_rux stats in proc0_post
rGdafe50ebd874: thread0: Clear td_rux stats in proc0_post

Summary

proc0_post aims to reset the CPU usage accounting for all threads and
processes in the system to zero once the time of day is verified.
However, the per-thread stats were not being cleared, resulting in
over-reported time for thread0 post-boot.

Fixes: bed4c5241663 ("Implement RUSAGE_THREAD. Add td_rux...")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 69004
Build 65887: arc lint + arc unit

Event Timeline

jhb created this revision.Tue, Dec 2, 10:44 PM

Herald added subscribers: olce, imp. · View Herald TranscriptTue, Dec 2, 10:44 PM

jhb requested review of this revision.Tue, Dec 2, 10:44 PM

Harbormaster completed remote builds in B69004: Diff 167444.Tue, Dec 2, 10:44 PM

jhb added a parent revision: D54039: thread0: Stop calling thread0 "swapper".Tue, Dec 2, 10:44 PM

markj accepted this revision.Tue, Dec 2, 11:14 PM

markj added inline comments.

sys/kern/init_main.c
662	This comment is somewhat misleading.

This revision is now accepted and ready to land.Tue, Dec 2, 11:14 PM

kib accepted this revision.Tue, Dec 2, 11:51 PM

kib added inline comments.

sys/kern/init_main.c
669	Don't we need both process stat spinlock (already taken) and thread lock to safely modify td_rux?

olce added inline comments.Wed, Dec 3, 9:18 AM

sys/kern/init_main.c
668–672	The reset to 0 could be factored out in a new `ruxreset_locked()` function taking a pointer to a `struct rusage_ext`, allowing to replace both these assignments and those before the loop by function calls.
669	Don't we need both process stat spinlock (already taken) and thread lock to safely modify td_rux? Yes.

jhb added inline comments.Wed, Dec 3, 3:38 PM

sys/kern/init_main.c
662	I almost removed it, but rufetch() does clear some other thread stats (td_[usi]ticks for example).
668–672	Except it's not a full reset. Maybe we should also be clearing rux_[ust]u as well? They are probably zero since nothing can query them this early in practice.
669	Mmm, yes.

olce added inline comments.Wed, Dec 3, 4:13 PM

sys/kern/init_main.c
668–672	That seems to make more sense yes, these fields are somewhat related, in particular having `rux_runtime` reset to 0 and non-zero values for `rux_[ust]u` is inconsistent.