svn commit: r349029 - head/sys/kern

[Alexander Motin]

Hi Bruce,

On 14.06.2019 08:58, Bruce Evans wrote:
> On Fri, 14 Jun 2019, Alexander Motin wrote:
> 
>> Log:
>>  Update td_runtime of running thread on each statclock().
>>
>>  Normally td_runtime is updated on context switch, but there are some
>> kernel
>>  threads that due to high absolute priority may run for many seconds
>> without
>>  context switches (yes, that is bad, but that is true), which means their
>>  td_runtime was not updated all that time, that made them invisible
>> for top
>>  other then as some general CPU usage.
>>
>>  MFC after:    1 week
>>  Sponsored by:    iXsystems, Inc.
> 
> This and more is supposed to be done in calcru().  It is also necessary to
> adjust for the current timeslice.
> 
> I thought that calcru() was fixed, but the fix seems to be only in my
> version of FreeBSD-5.2.
> 
> The bug seems to be in fill_kinfo_proc_only().  calcru() should update
> td_runtime for all threads in the proc (and sum into rux_rutime),

...

> td_runtime is updated in one other place: in rufetchtd(), but this function
> has the same bug eas everywhere else -- it only updates the runtimes for
> curthread.

I think it has very simple reason -- now each CPU measures CPU time in
its own time units, since cpu_ticks() are not synchronized and can not
be compared between different CPUs, so to update td_runtime of another
running thread you would need to switch to that CPU, or somehow else get
the value (cache it periodically?).

I see in your code you are using binuptime() calls instead of
cpu_ticks().  I suppose it fixes many problems, since it is globally
synchronous, but there was a reason why cpu_ticks() exists -- it is
cheaper on system non-synchronized TSC.  May be we could reconsider
that, giving up on old platforms, expecting all new one to not have this
problem, but for right now I think my patch is good enough to make top
sane again, that annoyed me for years.

-- 
Alexander Motin