From owner-svn-src-all@freebsd.org Fri Dec 9 18:56:38 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6229CC6FEB5; Fri, 9 Dec 2016 18:56:38 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 322881AE7; Fri, 9 Dec 2016 18:56:37 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id uB9IuaSt036663 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 9 Dec 2016 10:56:36 -0800 (PST) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id uB9IuaGY036662; Fri, 9 Dec 2016 10:56:36 -0800 (PST) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 9 Dec 2016 10:56:36 -0800 From: Gleb Smirnoff To: Konstantin Belousov 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> References: <201612091758.uB9HwYC0087384@repo.freebsd.org> <20161209184117.GJ54029@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161209184117.GJ54029@kib.kiev.ua> User-Agent: Mutt/1.7.0 (2016-08-17) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Dec 2016 18:56:38 -0000 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.