RFR: 8254264: Remove redundant cross_modify_fence()

[David Holmes]

<trimming>

On 17/10/2020 4:33 am, Patricio Chilano wrote:
> On 10/16/20 1:58 AM, David Holmes wrote:
>> So my only uncertainty now is the _thread_in_native case for 
>> suspension. But how can we be _thread_in_native at the time we call 
>> java_suspend_self_with_safepoint_check? When we call from 
>> check_safepoint_and_suspend_for_native_trans we are in 
>> _thread_in_native_trans. So that leaves 
>> handle_special_runtime_exit_condition, which is called from:
>>
>> - JavaCallWrapper::JavaCallWrapper
>>   - thread must be _thread_in_vm here
>>
>> - SafepointSynchronize::block
>>   - can't be _thread_in_native (fatal error)
>>
>> - ~ThreadInVMfromJava()
>>  - thread must be _thread_in_Java
>>
>> - ThreadToNativeFromVM
>>   - thread must be _thread_in_vm
>>
>> - ~ThreadInVMfromJavaNoAsyncException
>>   - thread must be _thread_in_vm
>>
>> So as far as I can see we can never be _thread_in_native when calling 
>> JavaThread::java_suspend_self_with_safepoint_check. 
> Only with ThreadToNativeFromVM, because we switch to native and then 
> call has_special_runtime_exit_condition().

Doh! Yes that was a silly oversight on my part.

Thanks,
David
-----

  Which by the way, I don't see
> why we need to make that call since we are going from vm->native. We 
> don't deliver async exceptions and we don't need to check for suspend 
> since we are going to native. If we remove that call we can remove that 
> conditional in java_suspend_self_with_safepoint_check() and always call 
> SafepointMechanism::process_if_requested().
> 
>> So it seems I don't have to worry about that case after all :) (and in 
>> any case when leaving _thread_in_native we will call 
>> check_safepoint_and_suspend_for_native_trans so we'd get to the 
>> cross-modify-fence via that route.)
> Exactly.
> 
>> \So long story short: this all looks good to me. :)
> Thanks David!
> 
> 
> Patricio
>> Thanks,
>> David
>> -----
>>
>>
>>
>>>
>>> Patricio
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> Patricio
>>>>>
>>>>> -------------
>>>>>
>>>>> Commit messages:
>>>>>   - v1
>>>>>
>>>>> Changes: https://git.openjdk.java.net/jdk/pull/655/files
>>>>>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=655&range=00
>>>>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8254264
>>>>>    Stats: 5 lines in 2 files changed: 0 ins; 5 del; 0 mod
>>>>>    Patch: https://git.openjdk.java.net/jdk/pull/655.diff
>>>>>    Fetch: git fetch https://git.openjdk.java.net/jdk 
>>>>> pull/655/head:pull/655
>>>>>
>>>>> PR: https://git.openjdk.java.net/jdk/pull/655
>>>>>
>>>
>