Date: Fri, 9 Dec 2016 10:56:36 -0800 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> 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: <20161209185636.GJ27748@FreeBSD.org> In-Reply-To: <20161209184117.GJ54029@kib.kiev.ua> References: <201612091758.uB9HwYC0087384@repo.freebsd.org> <20161209184117.GJ54029@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin,
On Fri, Dec 09, 2016 at 08:41:17PM +0200, Konstantin Belousov wrote:
K> > +int64_t
K> > +counter_ratecheck(struct counter_rate *cr, int64_t limit)
K> > +{
K> > + int64_t val;
K> > + int now;
K> > +
K> > + val = cr->cr_over;
K> > + now = ticks;
K> > +
K> > + if (now - cr->cr_ticks >= hz) {
K> > + /*
K> > + * Time to clear the structure, we are in the next second.
K> > + * First try unlocked read, and then proceed with atomic.
K> > + */
K> > + if ((cr->cr_lock == 0) &&
K> > + atomic_cmpset_int(&cr->cr_lock, 0, 1)) {
K> This should be cmpset_acq to avoid reordering of the operations from
K> locked region outside of it, and to make _rel below to sync-with.
K> Or call atomic_thread_fence_acq() after cmpset(), then unneeded barrier
K> is not executed if atomic failed.
Thanks!
K> > + /*
K> > + * Check if other thread has just went through the
K> > + * reset sequence before us.
K> > + */
K> > + if (now - cr->cr_ticks >= hz) {
K> > + val = counter_u64_fetch(cr->cr_rate);
K> > + counter_u64_zero(cr->cr_rate);
K> > + cr->cr_over = 0;
K> > + cr->cr_ticks = now;
K> > + }
K> > + atomic_store_rel_int(&cr->cr_lock, 0);
K> > + } else
K> > + /*
K> > + * We failed to lock, in this case other thread may
K> > + * be running counter_u64_zero(), so it is not safe
K> > + * to do an update, we skip it.
K> > + */
K> > + return (val);
K> > + }
K> > +
K> > + counter_u64_add(cr->cr_rate, 1);
K> What prevents this thread to fail the check for
K> now - cr->cr_ticks >= hz
K> above, then get off the cpu long enough for the next second to tick,
K> so that another thread starts the cleanup, while this thread performs the
K> increment ? The current thread update is lost then.
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.
--
Totus tuus, Glebius.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161209185636.GJ27748>