Re: [PATCH v3 1/2] x86: restore pv_rtc_handler() invocation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.07.2020 16:51, Roger Pau Monné wrote:
> On Wed, Jul 15, 2020 at 03:51:17PM +0200, Jan Beulich wrote:
>> On 15.07.2020 15:32, Roger Pau Monné wrote:
>>> Feel free to change to ACCESS_ONCE or barrier if you think it's
>>> clearer.
>>
>> I did so (also on the writer side), not the least based on guessing
>> what Andrew would presumably have preferred.
> 
> Thanks! Sorry I might be pedantic, but is the ACCESS_ONCE on the write
> side actually required? I'm not sure I see what ACCESS_ONCE protects
> against in handle_rtc_once.

Well, this is all sort of a mess, I think. We have this mixture of
ACCESS_ONCE() and read_atomic() / write_atomic(), but I don't think
we use them consistently, and I'm not sure either is suitable to
deal with all (theoretical) corner cases.

read_atomic() / write_atomic() guarantee a single insn to be used
to access a piece of data. I'm uncertain whether they also guarantee
single access (i.e. that the compiler won't replicate the asm()-s).
The wording in gcc doc is pretty precise, but not quite enough imo
to be entirely certain.

ACCESS_ONCE() guarantees single access, but doesn't guarantee that
the compiler wouldn't split this single access into multiple insns.
(It's just, like elsewhere, that it would be pretty silly of it if
it did.)

Yesterday, as said, I tried to in particular do what I expect/guess
Andrew would have wanted done. This is despite me not being entirely
convinced this is the right thing to do here, i.e. personally I
would have preferred read_atomic() / write_atomic(), as I think the
intention of what the gcc doc is saying is what we want (taking
into consideration both uses of "volatile" in these helpers).

Jan