Date: Fri, 14 Jun 2019 09:46:25 -0400 From: Alexander Motin <mav@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r349029 - head/sys/kern Message-ID: <399ec4d3-6548-1145-4dea-f0b7850f8381@FreeBSD.org> In-Reply-To: <20190614214154.I1201@besplex.bde.org> References: <201906140109.x5E19Aj9087899@repo.freebsd.org> <20190614214154.I1201@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?399ec4d3-6548-1145-4dea-f0b7850f8381>