Date: Sat, 10 Dec 2016 23:13:25 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r309745 - in head: share/man/man9 sys/kern sys/sys Message-ID: <20161210211325.GT54029@kib.kiev.ua> In-Reply-To: <20161209185636.GJ27748@FreeBSD.org> References: <201612091758.uB9HwYC0087384@repo.freebsd.org> <20161209184117.GJ54029@kib.kiev.ua> <20161209185636.GJ27748@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 09, 2016 at 10:56:36AM -0800, Gleb Smirnoff wrote: > Yes, this is expected. The interface isn't designed to be precise. So if > we hit limit, the next second result will be 20345 events exceeded the rate > instead of 20346 events. Note that the interface may legitimately give errors in the other direction as well, see below. The initial read of cr_ticks is racy, so one thread noting the condition of >= hz and executing counter_u64_zero() might race with another thread not observing the condition and executing counter_u64_add() in parallel. I believe this is possible even on relatively ordered arches like x86. Now, since counter_u64_add() is not atomic for the pcpu cell update (e.g. on x86), increment of the current pcpu cell might race with zeroing it, resulting in the old value of the counter cell to survive the change of ticks second. The end result is that events counted in the previous second would be transferred to the next second despite the reset, and cause unmerited rate check trigger. At very least, the function annotation should contain a warning to use it only for informational messages.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161210211325.GT54029>