From patchwork Mon Jan 11 11:28:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 8002581 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2EFCCBEEE5 for ; Mon, 11 Jan 2016 11:28:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4EF05202F0 for ; Mon, 11 Jan 2016 11:28:15 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 8815A20295 for ; Mon, 11 Jan 2016 11:28:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AC266E471; Mon, 11 Jan 2016 03:28:11 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id E42836E471 for ; Mon, 11 Jan 2016 03:28:06 -0800 (PST) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from nuc-i3427.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 51075001-1500048 for multiple; Mon, 11 Jan 2016 11:29:25 +0000 Received: by nuc-i3427.alporthouse.com (sSMTP sendmail emulation); Mon, 11 Jan 2016 11:28:02 +0000 Date: Mon, 11 Jan 2016 11:28:01 +0000 From: Chris Wilson To: Andy Lutomirski Subject: Re: [PATCH] x86: Add an explicit barrier() to clflushopt() Message-ID: <20160111112801.GR652@nuc-i3427.alporthouse.com> Mail-Followup-To: Chris Wilson , Andy Lutomirski , "H. Peter Anvin" , "linux-kernel@vger.kernel.org" , Ross Zwisler , "H . Peter Anvin" , Borislav Petkov , Brian Gerst , Denys Vlasenko , Linus Torvalds , Thomas Gleixner , Imre Deak , Daniel Vetter , DRI References: <1445248735-11915-1-git-send-email-chris@chris-wilson.co.uk> <20160107101652.GF652@nuc-i3427.alporthouse.com> <20160107194413.GA25144@nuc-i3427.alporthouse.com> <568ED31F.1090004@zytor.com> <20160107215401.GB25144@nuc-i3427.alporthouse.com> <568EE6E6.4000904@zytor.com> <568EE777.80700@zytor.com> <20160109080138.GG652@nuc-i3427.alporthouse.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Denys Vlasenko , Brian Gerst , "linux-kernel@vger.kernel.org" , DRI , Daniel Vetter , Borislav Petkov , "H. Peter Anvin" , Ross Zwisler , "H . Peter Anvin" , Linus Torvalds , Thomas Gleixner X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Jan 09, 2016 at 02:36:03PM -0800, Andy Lutomirski wrote: > On Sat, Jan 9, 2016 at 12:01 AM, Chris Wilson wrote: > > On Thu, Jan 07, 2016 at 02:32:23PM -0800, H. Peter Anvin wrote: > >> On 01/07/16 14:29, H. Peter Anvin wrote: > >> > > >> > I would be very interested in knowing if replacing the final clflushopt > >> > with a clflush would resolve your problems (in which case the last mb() > >> > shouldn't be necessary either.) > >> > > >> > >> Nevermind. CLFLUSH is not ordered with regards to CLFLUSHOPT to the > >> same cache line. > >> > >> Could you add a sync_cpu(); call to the end (can replace the final mb()) > >> and see if that helps your case? > > > > s/sync_cpu()/sync_core()/ > > > > No. I still see failures on Baytrail and Braswell (Pineview is not > > affected) with the final mb() replaced with sync_core(). I can reproduce > > failures on Pineview by tweaking the clflush_cache_range() parameters, > > so I am fairly confident that it is validating the current code. > > > > iirc sync_core() is cpuid, a heavy serialising instruction, an > > alternative to mfence. Is there anything that else I can infer about > > the nature of my bug from this result? > > No clue, but I don't know much about the underlying architecture. > > Can you try clflush_cache_ranging one cacheline less and then manually > doing clflushopt; mb on the last cache line, just to make sure that > the helper is really doing the right thing? You could also try > clflush instead of clflushopt to see if that makes a difference. I had looked at increasing the range over which clflush_cache_range() runs (using roundup/rounddown by cache lines), but it took something like +/- 256 bytes to pass all the tests. And also did s/clflushopt/clflush/ to confirm that made no differnce. Bizarrely, works like a charm. -Chris diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index 6000ad7..cf074400 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -141,6 +141,7 @@ void clflush_cache_range(void *vaddr, unsigned int size) for (; p < vend; p += clflush_size) clflushopt(p); + clflushopt(vend-1); mb(); } EXPORT_SYMBOL_GPL(clflush_cache_range);