RFR: 8254264: Remove redundant cross_modify_fence()

[Robbin Ehn]

On Fri, 16 Oct 2020 04:59:08 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review the following patch that removes some uneeded uses of cross_modify_fence() in common code, in particular
>> from ~ThreadBlockInVM(), ~ThreadBlockInVMWithDeadlockCheck() and java_suspend_self_with_safepoint_check(). These fences
>> were added before each JavaThread had to disarm itself (8230594). After a safepoint/handshake each JavaThread will
>> always call SafepointMechanism::process_if_requested_slow() when transitioning out of the safe state and will execute a
>> cross_modify_fence(). Tested with mach5 tiers1-7.  Thanks, Patricio
>
> Marked as reviewed by dholmes (Reviewer).

> > And just to be clear any action that changes code such that a
> > cross-modify-fence is needed by other threads, has to perform a global
> > safepoint/handshake?
> 
> I'm not sure of that. But we added these fences for the case where the
> code got patched during a safepoint. Maybe Robbin knows the answer.? : )

On x86 we have two complaint option according to the manual:

(* OPTION 1 *)
Store modified code (as data) into code segment;
Jump to new code or an intermediate location;
Execute new code;

(* OPTION 2 *)
Store modified code (as data) into code segment;
Execute a serializing instruction; (* For example, CPUID instruction *)
Execute new code;

- Code patching that happens outside a safepoint do options 1 and don't need CMF.
  If CMF is needed on some arch, it must be encoded into the instruction stream while patching.
  (whether this is really safe, thinking of spectre, is another story :) )
  See e.g. NativeGeneralJump::replace_mt_safe()
- Code patching that happens inside a safepoint do options 2 and need CMF.
  We actual only need CMF for safepoints, but since we do not know why we were armed we must emit it.
  So can be optimize by keeping track of the safepoint id and for which safepoint id we have emitted CMF for per java
  thread.

-------------

PR: https://git.openjdk.java.net/jdk/pull/655