From owner-svn-src-head@freebsd.org Thu Apr 25 05:38:34 2019 Return-Path: Delivered-To: svn-src-head@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 19481158D005 for ; Thu, 25 Apr 2019 05:38:34 +0000 (UTC) (envelope-from wma@semihalf.com) Received: from mail-it1-x12b.google.com (mail-it1-x12b.google.com [IPv6:2607:f8b0:4864:20::12b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A9BB26AFF4 for ; Thu, 25 Apr 2019 05:38:33 +0000 (UTC) (envelope-from wma@semihalf.com) Received: by mail-it1-x12b.google.com with SMTP id e13so10275360itk.4 for ; Wed, 24 Apr 2019 22:38:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MDx2VGmameH1C/S1ojWDxPsqka/2FqiBypmLlYHY8x4=; b=wCD6l6wds+QN741w83bbLWxyHrv2cKrsz7vNubIdCoZ8uPmgTu8xsUiRqN15mbIxff l8C4vGztBMEKN28CJM1zn/r05TS/gYra0sR7uoiAwtIPktG8WVchuyhszFaVOFgk0V8S h0mE2Y2LAXOXOfgMcBTSjgckndvds5csi+MCCBrH3fPV+iFi2QN7ZYRVxzCoL2tmrFwr G1riQQdjCxxy3RkEVc1Ec0M6RT92zMcNeppYNVghFL9a9SuZRish/NXFXr0AVCcd412n KNOc1QJc7e0rJUYGWiTH0lUKFVNH+n7ESGMXsONgMNTheHCieXLGicuR2T4moC+/76X7 noQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MDx2VGmameH1C/S1ojWDxPsqka/2FqiBypmLlYHY8x4=; b=j3u2EM8VA82VmgAjr0gUQzHDm56nq3Spm61cTugp7k6aWLLULUfl33kP1ngsoSJxTj wVmb3j3Hz1XndyyHC7I9SzN9ysF/v9z1pEsSHSp8j41r5HIi2t1b6NwEnq7Aaki1jG3n v1zpdECCFRYS7WFL3IPzSHvRvZyD3wvPYscDVQlr19vI/NKgf76CcaLsrUhl219yvbUb PjekVcXj7kQB/WFr/XZqA8i1igaty5j5oyB1tsZA/uMnPSAyrUtzYsaCKEHj9mbgUY5o +bih03/sUgi2Gs9FdIlxier/TnXsa8uoAyi8phlBSeUA+xBCEpy9B4+EodDcy3X5QfU2 syWg== X-Gm-Message-State: APjAAAWowpb3kotaevvyuP31wdIp7ub9mCglW0Wx2yam5qpnofjBta9F gXr6aSTFv7ABB3If+lMUi7YAoBJfA7S6yypzI7rUJQ== X-Google-Smtp-Source: APXvYqzUMwvWGrN5YfeRPAo5N1W3BcVYJWVrMpnd8EG9V9Z0b6kzuKAashR36tXBa7CyaMNvc00oEu0Z0AZvqmNZ1g4= X-Received: by 2002:a24:5905:: with SMTP id p5mr2483274itb.171.1556170712456; Wed, 24 Apr 2019 22:38:32 -0700 (PDT) MIME-Version: 1.0 References: <201904230636.x3N6aWQK057863@repo.freebsd.org> <20190425040817.GA3789@spy> In-Reply-To: <20190425040817.GA3789@spy> From: Wojciech Macek Date: Thu, 25 Apr 2019 07:38:21 +0200 Message-ID: Subject: Re: svn commit: r346593 - head/sys/sys To: Mark Johnston Cc: Wojciech Macek , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org X-Rspamd-Queue-Id: A9BB26AFF4 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.92 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.92)[-0.920,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Apr 2019 05:38:34 -0000 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. Wojtek czw., 25 kwi 2019 o 06:08 Mark Johnston napisa=C5=82(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 > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > --- 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 =3D=3D 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 fetch= ed > > + * 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? >