From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757397AbbFQPQf (ORCPT ); Wed, 17 Jun 2015 11:16:35 -0400 Received: from www.linutronix.de ([62.245.132.108]:49725 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755216AbbFQPQd (ORCPT ); Wed, 17 Jun 2015 11:16:33 -0400 Date: Wed, 17 Jun 2015 17:15:57 +0200 (CEST) From: Thomas Gleixner To: Andy Lutomirski cc: Dan Williams , Arnd Bergmann , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Ross Zwisler , Andrew Morton , Juergen Gross , X86 ML , Toshi Kani , linux-nvdimm , Benjamin Herrenschmidt , Luis Rodriguez , Konrad Rzeszutek Wilk , "linux-kernel@vger.kernel.org" , Stefan Bader , "linux-mm@kvack.org" , Geert Uytterhoeven , Ralf Baechle , Henrique de Moraes Holschuh , Michael Ellerman , Tejun Heo , Paul Mackerras , Christoph Hellwig Subject: Re: [PATCH v4 6/6] arch, x86: pmem api for ensuring durability of persistent memory updates In-Reply-To: Message-ID: References: <20150611211354.10271.57950.stgit@dwillia2-desk3.amr.corp.intel.com> <20150611211947.10271.80768.stgit@dwillia2-desk3.amr.corp.intel.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001,URIBL_BLOCKED=0.001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 17 Jun 2015, Andy Lutomirski wrote: > On Thu, Jun 11, 2015 at 2:19 PM, Dan Williams wrote: > > +static inline void arch_sync_pmem(void) > > +{ > > + wmb(); > > + pcommit_sfence(); > > +} > > This function is non-intuitive to me. It's really "arch-specific sync > pmem after one or more copies using arch_memcpy_to_pmem". If normal > stores or memcpy to non-WC memory is used instead, then it's > insufficient if the memory is WB and it's unnecessarily slow if the > memory is WT or UC (the first sfence isn't needed). > > I would change the name and add documentation. I'd also add a comment > about the wmb() being an SFENCE to flush pending non-temporal writes. Not "I'd also add ...". Documentation of memory barriers are mandatory. Thanks, tglx From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46]) by kanga.kvack.org (Postfix) with ESMTP id E9FE56B0072 for ; Wed, 17 Jun 2015 11:16:36 -0400 (EDT) Received: by wgv5 with SMTP id 5so39854036wgv.1 for ; Wed, 17 Jun 2015 08:16:36 -0700 (PDT) Received: from Galois.linutronix.de (Galois.linutronix.de. [2001:470:1f0b:db:abcd:42:0:1]) by mx.google.com with ESMTPS id m6si9677796wif.81.2015.06.17.08.16.35 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 17 Jun 2015 08:16:35 -0700 (PDT) Date: Wed, 17 Jun 2015 17:15:57 +0200 (CEST) From: Thomas Gleixner Subject: Re: [PATCH v4 6/6] arch, x86: pmem api for ensuring durability of persistent memory updates In-Reply-To: Message-ID: References: <20150611211354.10271.57950.stgit@dwillia2-desk3.amr.corp.intel.com> <20150611211947.10271.80768.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: Andy Lutomirski Cc: Dan Williams , Arnd Bergmann , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Ross Zwisler , Andrew Morton , Juergen Gross , X86 ML , Toshi Kani , linux-nvdimm , Benjamin Herrenschmidt , Luis Rodriguez , Konrad Rzeszutek Wilk , "linux-kernel@vger.kernel.org" , Stefan Bader , "linux-mm@kvack.org" , Geert Uytterhoeven , Ralf Baechle , Henrique de Moraes Holschuh , Michael Ellerman , Tejun Heo , Paul Mackerras , Christoph Hellwig On Wed, 17 Jun 2015, Andy Lutomirski wrote: > On Thu, Jun 11, 2015 at 2:19 PM, Dan Williams wrote: > > +static inline void arch_sync_pmem(void) > > +{ > > + wmb(); > > + pcommit_sfence(); > > +} > > This function is non-intuitive to me. It's really "arch-specific sync > pmem after one or more copies using arch_memcpy_to_pmem". If normal > stores or memcpy to non-WC memory is used instead, then it's > insufficient if the memory is WB and it's unnecessarily slow if the > memory is WT or UC (the first sfence isn't needed). > > I would change the name and add documentation. I'd also add a comment > about the wmb() being an SFENCE to flush pending non-temporal writes. Not "I'd also add ...". Documentation of memory barriers are mandatory. Thanks, tglx -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org