Re: [PATCH 3/4] x86,asm: Re-work smp_store_mb()

From: Linus Torvalds
Date: Tue Jan 12 2016 - 13:04:23 EST


On Tue, Jan 12, 2016 at 9:45 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>
> By the way, the comment in barrier.h says:
>
> /*
> * Some non-Intel clones support out of order store. wmb() ceases to be
> * a nop for these.
> */
>
> and while the 1st sentence may well be true, if you have
> an SMP system with out of order stores, making wmb
> not a nop will not help.
>
> Additionally as you point out, wmb is not a nop even
> for regular intel CPUs because of these weird use-cases.
>
> Drop this comment?

We should drop it, yes. We dropped support for CONFIG_X86_OOSTORE
almost two years ago. See commit 09df7c4c8097 ("x86: Remove
CONFIG_X86_OOSTORE") and it was questionable for a long time even
before that (perhaps ever).

So the comment is stale.

We *do* still use the non-nop rmb/wmb for IO barriers, but even that
is generally questionable. See our "copy_user_64.S" for an actual use
of "movnt" followed by sfence. There's a couple of other cases too. So
that's all correct, but the point is that when we use "movnt" we don't
actually use "wmb()", we are doing assembly, and the assembly should
just use sfence directly.

So it's actually very questionable to ever make even the IO
wmb()/rmb() functions use lfence/sfence. They should never really need
it.

But at the same time, I _really_ don't think we care enough. I'd
rather leave those non-smp barrier cases alone as historial unless
somebody can point to a case where they care about the performance.

We also do have the whole PPRO_FENCE thing, which we can hopefully get
rid of at some point too.

Linus