* Borislav Petkov <[email protected]> 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 <[email protected]>
> > Cc: H Peter Anvin <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
