Re: [Xen-devel] [PATCH v2 2/4] xen/x86: Drop unnecessary barriers

[Tim Deegan <tim@xxxxxxx>]

At 09:04 -0600 on 18 Aug (1503047077), Jan Beulich wrote:
> >>> On 18.08.17 at 16:47, <tim@xxxxxxx> wrote:
> > At 01:48 -0600 on 17 Aug (1502934495), Jan Beulich wrote:
> >> >>> On 16.08.17 at 18:47, <andrew.cooper3@xxxxxxxxxx> wrote:
> >> > On 16/08/17 16:23, Jan Beulich wrote:
> >> >>>>> On 16.08.17 at 13:22, <andrew.cooper3@xxxxxxxxxx> wrote:
> >> >>> --- a/xen/arch/x86/mm/shadow/multi.c
> >> >>> +++ b/xen/arch/x86/mm/shadow/multi.c
> >> >>> @@ -3112,7 +3112,6 @@ static int sh_page_fault(struct vcpu *v,
> >> >>>       * will make sure no inconsistent mapping being translated into
> >> >>>       * shadow page table. */
> >> >>>      version = 
> >> >>> atomic_read(&d->arch.paging.shadow.gtable_dirty_version);
> >> >>> -    rmb();
> >> >>>      walk_ok = sh_walk_guest_tables(v, va, &gw, error_code);
> >> >> Isn't this supposed to make sure version is being read first? I.e.
> >> >> doesn't this at least need to be barrier()?
> >> > 
> >> > atomic_read() is not free to be reordered by the compiler.  It is an asm
> >> > volatile with a volatile memory reference.
> >> 
> >> Oh, right - I did forget about the volatiles there (since generally,
> >> like in Linux, we appear to try to avoid volatile).
> > 
> > FWIW, I don't think that's quite right.  The GCC docs I have say that
> > "volatile" will stop the compiler from omitting an asm altogether, or
> > hoisting it out of a loop (on the assumption that it will always
> > produce the same output for the same inputs).  And that "the compiler
> > can move even volatile 'asm' instructions relative to other code,
> > including across jump instructions."
> 
> Oh, I had talked about the volatile qualifiers, no the volatile asm-s.

I'm not sure what other volatile you mean here, but accesses to
volatile objects are only ordered WRT other _volatile_ accesses.
So, e.g.: https://godbolt.org/g/L2qa8h

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel