From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753610AbbCLK6u (ORCPT ); Thu, 12 Mar 2015 06:58:50 -0400 Received: from mail-wg0-f41.google.com ([74.125.82.41]:42328 "EHLO mail-wg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752715AbbCLK6s (ORCPT ); Thu, 12 Mar 2015 06:58:48 -0400 Date: Thu, 12 Mar 2015 11:58:43 +0100 From: Ingo Molnar To: Borislav Petkov Cc: Ross Zwisler , linux-kernel@vger.kernel.org, H Peter Anvin , Thomas Gleixner Subject: Re: [PATCH] x86: Add kerneldoc for pcommit_sfence() Message-ID: <20150312105843.GA6812@gmail.com> References: <1426097961-24921-1-git-send-email-ross.zwisler@linux.intel.com> <20150311201830.GD3359@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150311201830.GD3359@pd.tnic> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Borislav Petkov wrote: > On Wed, Mar 11, 2015 at 12:19:21PM -0600, Ross Zwisler wrote: > > Add kerneldoc comments for pcommit_sfence() describing the purpose of > > the pcommit instruction and demonstrating the usage of that instruction. > > > > Signed-off-by: Ross Zwisler > > Cc: H Peter Anvin > > Cc: Ingo Molnar > > Cc: Thomas Gleixner > > Cc: Borislav Petkov > > --- > > arch/x86/include/asm/special_insns.h | 37 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h > > index aeb4666e0c0a..1ae81757c05b 100644 > > --- a/arch/x86/include/asm/special_insns.h > > +++ b/arch/x86/include/asm/special_insns.h > > @@ -215,6 +215,43 @@ static inline void clwb(volatile void *__p) > > : [pax] "a" (p)); > > } > > > > +/** > > + * pcommit_sfence() - persistent commit and fence > > + * > > + * The pcommit instruction ensures that data that has been flushed from the > > + * processor's cache hierarchy with clwb, clflushopt or clflush is accepted to > > + * memory and is durable on the DIMM. The primary use case for this is > > + * persistent memory. Please capitalize canonical instruction names like the CPU makers do, so that they stand out better in free flowing English text, i.e. something like: * * The PCOMMIT instruction ensures that data that has been flushed from the * processor's cache hierarchy with CLWB, CLFLUSHOPT or CLFLUSH is accepted to * memory and is durable on the DIMM. The primary use case for this is * persistent memory. * > > + * This function shows how to properly use clwb/clflushopt/clflush and pcommit > > + * with appropriate fencing: Ditto. > > + * > > + * void flush_and_commit_buffer(void *vaddr, unsigned int size) > > + * { > > + * unsigned long clflush_mask = boot_cpu_data.x86_clflush_size - 1; > > + * char *vend = (char *)vaddr + size; So here we cast vaddr to (char *) - which is unnecessary, as 'void *' has byte granular pointer arithmetics. And 'vend' should be void *' to begin with, to match the type of 'vaddr'. > > + * char *p; Ditto. > > + * > > + * for (p = (char *)((unsigned long)vaddr & ~clflush_mask); > > + * p < vend; p += boot_cpu_data.x86_clflush_size) > > + * clwb(p); > > + * > > + * // sfence to order clwb/clflushopt/clflush cache flushes > > + * // mfence via mb() also works Yeah so this isn't a C++ kernel, thank all the 3000+ gods and other supreme beings worshipped on this planet! > > + * wmb(); > > + * > > + * // pcommit and the required sfence for ordering Ditto. > > + * pcommit_sfence(); > > + * } > > + * > > + * After this function completes the data pointed to by vaddr is has been > > s/is // > > I fixed it up while applying, Also please put 'vaddr' into single quotes, to make the parameter name stand out better in written text: > > + * After this function completes the data pointed to by 'vaddr' has been Thanks, Ingo