LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: "H. Peter Anvin" <h.peter.anvin@intel.com>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2 0/2] add support for new persistent memory instructions
Date: Mon, 26 Jan 2015 22:34:57 +0100	[thread overview]
Message-ID: <20150126213457.GI1656@pd.tnic> (raw)
In-Reply-To: <1422302369.13382.3.camel@theros.lm.intel.com>

On Mon, Jan 26, 2015 at 12:59:29PM -0700, Ross Zwisler wrote:
> This is interesting!  I guess I'm confused as to how this solves the ordering
> issue, though.  The "m" input vs "+m" output parameter will tell gcc whether
> or not the assembly can be reordered at compile time with respect to reads at
> that same location, correct?
> 
> So if we have an inline function that could either read or write from gcc's
> point of view (input vs output parameter, depending on the branch), it seems
> like it would be forced to fall back to the most restrictive case (assume it
> will write), and not reorder with respect to reads.  If so, you'd end up in
> the same place as using "+m" output, only now you've got an additional branch
> instead of a 3-way alternative.
> 
> Am I misunderstanding this?

No, you're not, that is the right question. I was simply hypothesizing
about how we could do what hpa suggests but I don't have any other ideas
about having an "m" and an "+m" in the same inline asm statement.

My hunch is, the moment we have an "+m", the reordering would be
suppressed and that would not give us the CLWB case where we don't have
to suppress reordering wrt reads.

> Ah, yep, I definitely need to include an example flow in my commit comments.
> :) Here's a snip from my reply to hpa, to save searching:
> 
> 	Both the flushes (wmb/clflushopt/clflush) and the pcommit are ordered
> 	by either mfence or sfence.
> 
> 	An example function that flushes and commits a buffer could look like
> 	this (based on clflush_cache_range):
> 
> 	void flush_and_commit_buffer(void *vaddr, unsigned int size)
> 	{       
> 		void *vend = vaddr + size - 1;
> 		
> 		for (; vaddr < vend; vaddr += boot_cpu_data.x86_clflush_size)
> 			clwb(vaddr);
> 		
> 		/* Flush any possible final partial cacheline */
> 		clwb(vend);
> 		
> 		/* 
> 		 * sfence to order clwb/clflushopt/clflush cache flushes
> 		 * mfence via mb() also works 
> 		 */
> 		wmb();
> 
> 		pcommit();

Oh, so you need an SFENCE to flush out the preceding in-flight writes
*and* PCOMMIT for the persistent memory ranges. Ok, makes sense, PCOMMIT
deals with the persistent stores.

> 		/* 
> 		 * sfence to order pcommit
> 		 * mfence via mb() also works 
> 		 */
> 		wmb();

Doc says PCOMMIT is not ordered wrt loads and SFENCE too. Don't we want
to be absolutely conservative here and use MFENCE both times?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

  reply	other threads:[~2015-01-26 21:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 20:40 Ross Zwisler
2015-01-23 20:40 ` [PATCH v2 1/2] x86: Add support for the pcommit instruction Ross Zwisler
2015-01-23 20:40 ` [PATCH v2 2/2] x86: Add support for the clwb instruction Ross Zwisler
2015-01-23 23:03 ` [PATCH v2 0/2] add support for new persistent memory instructions H. Peter Anvin
2015-01-24 11:14   ` Borislav Petkov
2015-01-26 19:59     ` Ross Zwisler
2015-01-26 21:34       ` Borislav Petkov [this message]
2015-01-26 21:50         ` Ross Zwisler
2015-01-26 22:39           ` Borislav Petkov
2015-01-26 23:14             ` Ross Zwisler
2015-01-26 19:51   ` Ross Zwisler
2015-01-26 20:05     ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150126213457.GI1656@pd.tnic \
    --to=bp@alien8.de \
    --cc=h.peter.anvin@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=ross.zwisler@linux.intel.com \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH v2 0/2] add support for new persistent memory instructions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).