RFR(XS): VM times out in VM_HandshakeAllThreads::doit() with RunThese30M
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Aug 15 20:34:02 UTC 2018
On 8/15/18 4:21 PM, Robbin Ehn wrote:
> Hi Dan, thanks,
>
> On 2018-08-15 22:02, Daniel D. Daugherty wrote:
>> So if HandshakeState::vmthread_can_process_handshake() is only
>> called by the VMThread while at a safepoint, then the target
>> JavaThread cannot be externally resumed while that function is
>> running so this new check:
>
> No we are not at safepoint, this method is never called at safepoint.
Okay I definitely misinterpreted what you were saying in this
email thread... let's try this again.
> All
> other threads should be considered running. Therefore I need to stop
> resuming targeted suspended thread while VM thread is executing the
> handshake on behalf of the suspended thread.
>
> Does my comment/assert make more sense in the light of that?
Hmmm.... Here's your comment:
+ // We must block the S/R protocol to resume a thread while the vm thread
+ // is executing the operation, by holding the Threads_lock.
+ // This is problematic, since we now can't remove the Threads_lock in
handshakes.
Mostly, but I still suggest a rewrite:
// SafepointSynchronize::safepoint_safe() does not consider an externally
// suspended thread to be safe. However, this function must be called
with
// the Threads_lock held so an externally suspended thread cannot be
// resumed thus it is safe.
Of course, this comment assumes that the Threads_lock is held in the caller
over the appropriate interval of code...
Dan
>
> /Robbin
>
>>
>> + return SafepointSynchronize::safepoint_safe(target,
>> target->thread_state()) ||
>> + target->is_ext_suspended();
>>
>> is okay because that externally suspended thread is also safepoint safe.
>>
>> This assert:
>>
>> + assert(Threads_lock->owned_by_self(), "Not holding Threads_lock.");
>>
>> doesn't quite match your intent. If
>> HandshakeState::vmthread_can_process_handshake()
>> should only be called by the VMThread at a safepoint, then a different
>> check would be better:
>>
>> assert(SafepointSynchronize::is_at_safepoint() &&
>> Thread::current()->is_VM_thread(),
>> "must be VMThread at a safepoint");
>>
>> Perhaps this for your comment:
>>
>> // SafepointSynchronize::safepoint_safe() does not consider an
>> externally
>> // suspended thread to be safe. However, if we are at a safepoint,
>> then
>> // the externally suspended thread cannot be resumed so it is safe.
>>
>> Dan
>>
>>
>>>
>>> Thanks, Robbin
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>
>
More information about the hotspot-runtime-dev
mailing list