[8u] RFR: nativeInstruction atomicity and correctness of patching

[Andrew Haley]

On 30/03/16 20:53, Anton Kozlov wrote:

> On Wed, 2016-03-30 at 16:25 +0100, Andrew Haley wrote:
>> On 03/30/2016 04:15 PM, Anton Kozlov wrote:
>>> ICache::invalidate_word used here not to provide coherency between
>>> Icache and Dcache, but to maintain in-order visibillity of code
>>> modification.
>>
>> What exactly are you trying to synchronize?

> ICache::invalidate ensures that all CPUs see modifications of code.
> Data modification in trampoline should be visible in the same way,
> same time as code modification would. For this ICache::invalidate
> used, as it by implementation should synchornize dcaches among CPUs.

I don't think so.  In order for this to work, the *reader* would have
to execute some kind of fence too.  I don't think that's possible
without an inter-processor interrupt across all cores, and I think
that __clear_cache doesn't do that.  I certainly hope not.

>>> This is achieved by implementation of ICache::invalidate_word,
>>> which is just call to __clear_cache syscall wrapper.
>>
>> I don't think there's any point.  It's not instructions but data, so
>> ICache::invalidate_word is inappropriate.
> 
> Agree, ICache is bad name for that. How about direct __clear_cache
> call?

> Also, I will try to study implementation of __clear_cache to know,
> how exactly it's done.

I think you will find that it does not do what you think it does.  I'm
looking at linux/arch/arm/include/asm/cacheflush.h and I can't see
anything like that.  All I see is that do_cache_op calls
__flush_cache_user_range, and all that does is ensure that the I and D
caches are coherent.

> But for me it looks like if we drop cache maintance in
> NativeTrampolineCall::set_destination_mt_safe, we'll get race
> condition, and some thread could call old destination.

Yes.  Some thread could indeed call the old destination, but the key
is that it doesn't matter: the old routines are not evicted from the
code cache until at a safepoint, so they can still be called.  Indeed,
they may still be running on some cores.  And we patch out the first
instruction so that it traps.

Andrew.