[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    RE: RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out
From:       Mattis Castegren <mattis.castegren () oracle ! com>
Date:       2015-10-30 10:38:32
Message-ID: 8a5a84cd-c367-4499-8f97-e5fb3202b596 () default
[Download RAW message or body]

Hi

I agree that there may be things we could add to VM.info that we couldn't add in an \
hs_err file due to the fact that we are crashing. However, I really do believe that \
we should use the same code. The goal of the hs_err file is exactly the same as the \
goal of VM.info, to print as much information about the system as possible so that \
when we look at an hs_err file or the output from VM.info, we get as much information \
about the system as possible. That way we can reduce the need to go back and forth \
with the customer to ask for more information.

If we enumerate what we need in VM.info, the list would look very much like what we \
already do in VMError, and therefore I think we should use the same code. If we find \
something that we would want, that is not currently printed in hs_err files, I think \
we have two options:

1) Add it to hs_err files as well. If it is interesting when gathering customer \
information before a crash, it probably is interesting after a crash 2) If it for \
some reason cannot be captured when crashing, add it to the code with the condition \
that we don't print it during crash time.

I would therefore suggest the following plan of action

1) Review and push this fix to see if there are any technical issues with regards to \
thread safety, etc. I know David has had this in mind when doing the change. After \
that, we have a command that support can use. 2) Think about if there is any \
information missing, both from hs_err files and from VM.info. Here, we may want to \
ask support about if there is any additional information they need to ask for when \
they get this data. If possible, this information should be added to both hs_err \
files and VM.info, we don't want support to have to ask for VM.info if we already \
have an hs_err file. Of course, if there is anything Dev would want to see when \
analyzing crashes in testing, we should add that too.

Kind Regards
/Mattis

-----Original Message-----
From: David Holmes 
Sent: den 29 oktober 2015 23:20
To: Mattis Castegren; Coleen Phillimore; hotspot-runtime-dev@openjdk.java.net
Subject: Re: RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out

On 29/10/2015 11:36 PM, Mattis Castegren wrote:
> Some background: We have this command in JRockit. The information you gather when \
> you crash to give a good summary of what system you run on is pretty much exactly \
> the information you need to get a good summary on a system that has not crashed. \
> The JRockit command is extremely useful for support, and saves a lot of work going \
> back and forth asking about system information. 
> Also, if we write something new in the hs_err file, like if there has been any out \
> of memory errors, we often would want the same information in the VM.info output. \
> From my experience in Sustaining/Support, I can't think of any information you \
> would want in VM.info that you wouldn't also want in the hs_err file and the other \
> way around, apart from details about the crash (obviously). 
> I don't see a reason to exactly enumerate what information VM.info should provide. \
> From a sustaining/support perspective, we want a one-stop command to gather as much \
> useful information as possible, which is the same idea we have for the hs_err file.

The reason to enumerate what is required is to see where that information already \
exists and can be collected from. The VMError report has to be very careful about \
what it does and how it does it because of the fact we may have crashed and the \
general process state is indeterminate. A Dcmd can simply gather whatever information \
is required from the available sources in whatever order or format desired.

I have no problem with this command, just how it has been proposed to implement it.

David

> Kind Regards
> /Mattis
> 
> -----Original Message-----
> From: David Holmes
> Sent: den 29 oktober 2015 13:08
> To: Coleen Phillimore; hotspot-runtime-dev@openjdk.java.net
> Subject: Re: RFR: 8027429: Add diagnostic command VM.info to get 
> hs_err print-out
> 
> On 29/10/2015 10:02 PM, Coleen Phillimore wrote:
> > 
> > 
> > On 10/29/15 7:58 AM, David Holmes wrote:
> > > On 29/10/2015 9:26 PM, Coleen Phillimore wrote:
> > > > 
> > > > So I actually disagree.  I don't think there should be an 
> > > > additional separate mechanism to get the same information that we 
> > > > get with hs_err reporting.  I've wanted to have a feature like this for a \
> > > > long time. 
> > > > I pre-reviewed this change and I thought it looked good in general.
> > > > I didn't see the thread iteration problem that Fred refers to 
> > > > below, but I think the individual problems can be fixed.
> > > > 
> > > > The last thing I want is this code to be copied somewhere else.
> > > 
> > > Factored as needed not copied. VMError is not an "info" reporting 
> > > mechanism.
> > 
> > If you look at the things that are reported in each "STEP", there's a 
> > small amount of code and the order is important.
> 
> The order is less important in an "info" request I would think.
> 
> > I'd like the vm info to use the same order and report what it can do
> > safely.   Refactoring 5 lines of code into functions doesn't make sense.
> 
> I need to consider exactly what it is the "info" needs to report in 
> more detail. There are existing facilities (system properties, 
> management
> APIs) for various bits of runtime information, which VMError can't utilize but a \
> Dcmd can. 
> David
> 
> > Coleen
> > 
> > > 
> > > David
> > > 
> > > > Coleen
> > > > 
> > > > On 10/28/15 8:48 PM, David Holmes wrote:
> > > > > I agree with Fred, this kind of info reporting should not be 
> > > > > piggy-backed onto VMError handling for the reasons stated (and the 
> > > > > error handling logic is complicated enough as it is!). For things 
> > > > > like thread lists there are already safe management functions that 
> > > > > can/should be used.
> > > > > 
> > > > > Thanks,
> > > > > David H.
> > > > > 
> > > > > On 29/10/2015 3:29 AM, Frederic Parain wrote:
> > > > > > Hi David,
> > > > > > 
> > > > > > I haven't review all the code yet, but I'm concerned with the 
> > > > > > fact that the diagnostic command is calling VMError::report().
> > > > > > This method has been implemented to be executed in the particular 
> > > > > > context of fatal errors, and its usage while the VM is running 
> > > > > > normally seems dangerous.
> > > > > > 
> > > > > > For instance, VMError::report() consciously avoids grabbing locks 
> > > > > > because of the risk of deadlock during the error reporting.
> > > > > > The consequence is that some data structures are browsed in an 
> > > > > > unsafe way. One example: VMError::report() calls
> > > > > > Threads::print_on_error() which iterates over the thread list 
> > > > > > *without owning the Threads_lock*.
> > > > > > 
> > > > > > The implementation of the diagnostic command seems also to 
> > > > > > exclude a lot of reporting from the initial VMError::report() 
> > > > > > method. Have you considered implementing a new MT-safe reporting 
> > > > > > method rather than trying to modify the special VMError::report() 
> > > > > > methods? (Note that some code factorization between 
> > > > > > VMError::report() and this new method should be possible).
> > > > > > 
> > > > > > Thanks,
> > > > > > 
> > > > > > Fred
> > > > > > 
> > > > > > On 28/10/2015 17:18, david buck wrote:
> > > > > > > Hi!
> > > > > > > 
> > > > > > > Please review my change for this small enhancement.
> > > > > > > 
> > > > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8027429
> > > > > > > webrev: http://cr.openjdk.java.net/~dbuck/8027429_jdk9_01/
> > > > > > > 
> > > > > > > Cheers,
> > > > > > > -Buck
> > > > > > 
> > > > 
> > 


[prev in list] [next in list] [prev in thread] [next in thread]