From owner-svn-src-all@freebsd.org Thu Apr 25 08:22:33 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 19CF7159006D; Thu, 25 Apr 2019 08:22:33 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 06D4C6FDBC; Thu, 25 Apr 2019 08:22:31 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x3P8MNrG094223 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 25 Apr 2019 11:22:26 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x3P8MNrG094223 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x3P8MMPT094222; Thu, 25 Apr 2019 11:22:22 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 25 Apr 2019 11:22:22 +0300 From: Konstantin Belousov To: Wojciech Macek Cc: Mark Johnston , Wojciech Macek , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r346593 - head/sys/sys Message-ID: <20190425082222.GJ12936@kib.kiev.ua> References: <201904230636.x3N6aWQK057863@repo.freebsd.org> <20190425040817.GA3789@spy> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 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: Thu, 25 Apr 2019 08:22:33 -0000 On Thu, Apr 25, 2019 at 07:38:21AM +0200, Wojciech Macek wrote: > Intel does not reorder reads against the condition "if" here. I know for > sure that ARM does, but therestill might be some other architectures that > also suffers such behavior - I just don't have any means to verify. > I remember the discussion for rS302292 where we agreed that this kind of > patches should be the least impacting in perfomrance as possible. Adding > unconditional memory barrier causes significant performance drop on Intel, > where in fact, the issue was never seen. > Atomic_thread_fence_acq() is nop on x86, or rather, it is compiler memory barrier. If you need read/read fence on some architectures, I am sure that you need compiler barrier on all. > Wojtek > > czw., 25 kwi 2019 o 06:08 Mark Johnston napisał(a): > > > On Tue, Apr 23, 2019 at 06:36:32AM +0000, Wojciech Macek wrote: > > > Author: wma > > > Date: Tue Apr 23 06:36:32 2019 > > > New Revision: 346593 > > > URL: https://svnweb.freebsd.org/changeset/base/346593 > > > > > > Log: > > > This patch offers a workaround to buf_ring reordering > > > visible on armv7 and armv8. Similar issue to rS302292. > > > > > > Obtained from: Semihalf > > > Authored by: Michal Krawczyk > > > Approved by: wma > > > Differential Revision: https://reviews.freebsd.org/D19932 > > > > > > Modified: > > > head/sys/sys/buf_ring.h > > > > > > Modified: head/sys/sys/buf_ring.h > > > > > ============================================================================== > > > --- head/sys/sys/buf_ring.h Tue Apr 23 04:06:26 2019 (r346592) > > > +++ head/sys/sys/buf_ring.h Tue Apr 23 06:36:32 2019 (r346593) > > > @@ -310,14 +310,23 @@ buf_ring_peek_clear_sc(struct buf_ring *br) > > > if (!mtx_owned(br->br_lock)) > > > panic("lock not held on single consumer dequeue"); > > > #endif > > > - /* > > > - * I believe it is safe to not have a memory barrier > > > - * here because we control cons and tail is worst case > > > - * a lagging indicator so we worst case we might > > > - * return NULL immediately after a buffer has been enqueued > > > - */ > > > + > > > if (br->br_cons_head == br->br_prod_tail) > > > return (NULL); > > > + > > > +#if defined(__arm__) || defined(__aarch64__) > > > + /* > > > + * The barrier is required there on ARM and ARM64 to ensure, that > > > + * br->br_ring[br->br_cons_head] will not be fetched before the > > above > > > + * condition is checked. > > > + * Without the barrier, it is possible, that buffer will be fetched > > > + * before the enqueue will put mbuf into br, then, in the > > meantime, the > > > + * enqueue will update the array and the br_prod_tail, and the > > > + * conditional check will be true, so we will return previously > > fetched > > > + * (and invalid) buffer. > > > + */ > > > + atomic_thread_fence_acq(); > > > +#endif > > > > Why is it specific to ARM? > >