PING: RFR: JDK-8155730: HeapInfoDCmd should get Heap_lock

[Yasumasa Suenaga]

PING: Could you review it?

>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.02/


Yasumasa


On 2016/05/17 19:19, Yasumasa Suenaga wrote:
> PING: Could you review it?
> I need review from GC folks.
>
>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.02/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/05/10 22:25, Yasumasa Suenaga wrote:
>> On 2016/05/10 16:48, David Holmes wrote:
>>> On 9/05/2016 10:34 PM, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>>> Taking locks during VMError handling is problematic - we're executing
>>>>> from a signal handler and the VM is crashing. Much of what we do from
>>>>> VMError is "best effort".
>>>>
>>>> I will remove MutexLocker from VMError::report().
>>>> But I added assert_locked_or_safepoint(Heap_lock) in each CollectedHeap.
>>>> If we remove Heap_lock from VMError::report(), fastdebug HotSpot is failed
>>>> when it prints heap info to hs_err log.
>>>> Should I remove assert_locked_or_safepoint()?
>>>
>>> Yes I think you need to do this.
>>
>> I removed them in new webrev:
>>
>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.02/
>>
>>>> VMError::print_vm_info() calls from VMInfoDCmd::execute() only.
>>>> So I think we should take Heap_lock at this function.
>>>> Or should we take Heap_lock at VMInfoDCmd::execute() ?
>>>
>>> If this is called from a DCmd only then I think the lock can be taken in print_vm_info().
>>
>> I grep'ed HotSpot source, print_vm_info() is called from DCmd only.
>>
>>> Need GC folk to add their weight here.
>>
>> I'm waiting reviewer from GC folks :-)
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/05/09 21:26, David Holmes wrote:
>>>>> On 9/05/2016 10:20 PM, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I've sent review request for JDK-8155730.
>>>>>> However, I found caller of Universe::heap()->print_*() in vmError.cpp .
>>>>>> Thus I added MutexLocker(Ex) for Heap_lock to VMError::report() and
>>>>>> VMError::print_vm_info().
>>>>>
>>>>> Taking locks during VMError handling is problematic - we're executing
>>>>> from a signal handler and the VM is crashing. Much of what we do from
>>>>> VMError is "best effort".
>>>>>
>>>>> David
>>>>>
>>>>>> Could you review again?
>>>>>>
>>>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.01/
>>>>>>
>>>>>>
>>>>>>> This needs to be reviewed by the GC folk not runtime. Please direct
>>>>>>> followups to hotspot-gc-dev.
>>>>>>
>>>>>> Thanks, David!
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/05/06 13:11, David Holmes wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> This needs to be reviewed by the GC folk not runtime. Please direct
>>>>>>> followups to hotspot-gc-dev.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 30/04/2016 12:05 AM, Yasumasa Suenaga wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> This review request is related to [1].
>>>>>>>>
>>>>>>>> HeapInfoDCmd calls CollectedHeap::print_on().
>>>>>>>> However, its call does not get Heap_lock.
>>>>>>>>
>>>>>>>> Heap_lock should be gotten from all heap operations.
>>>>>>>>
>>>>>>>> I uploaded webrev for this issue.
>>>>>>>> Could you review it?
>>>>>>>>
>>>>>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.00/
>>>>>>>>
>>>>>>>> I cannot access JPRT.
>>>>>>>> So I need a sponsor.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019257.html
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>