RFR : 8222280 : Provide virtualization related info in the hs_error file on AIX

[Doerr, Martin]

Hi Matthias,

I agree with that the libperfstat_aix should be discussed separately.
It was needed for 2 reasons:
- Support as400/PASE where it is not available at all.
- Handle changes between AIX versions.

With jdk13, we currently only support AIX 7.2 so including the system header should be fine for now.
I can't estimate if there will be changes in future AIX versions which make our own lib important again.
But I suggest to remove our own libperfstat_aix in a follow up change with the risk that we may potentially have to add it back.
I'd like to hear Thomas' opinion on this, too.

We currently support neither as400 nor any other AIX, so your change looks good to me.

Best regards,
Martin


-----Original Message-----
From: ppc-aix-port-dev <ppc-aix-port-dev-bounces at openjdk.java.net> On Behalf Of Baesken, Matthias
Sent: Dienstag, 16. April 2019 13:52
To: ppc-aix-port-dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
Subject: [CAUTION] RE: RFR : 8222280 : Provide virtualization related info in the hs_error file on AIX

Ping - may I get a second review for this ?

Best regards, Matthias



> -----Original Message-----
> From: Baesken, Matthias
> Sent: Montag, 15. April 2019 09:10
> To: Langer, Christoph <christoph.langer at sap.com>; ppc-aix-port-
> dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net' <hotspot-
> dev at openjdk.java.net>
> Subject: RE: RFR : 8222280 : Provide virtualization related info in the hs_error
> file on AIX
> 
> Hi Christoph, thanks for the review .
> So I stay with the direct  usage  perfstat API in my change .
> 
> >
> > As regards to libperfstat usage: I think we should clean this up and try to
> link
> > during the build/use the system headers. We don't need to support AIX
> V5.3
> > as build release any longer and probably all required functionality is
> > contained in the headers of the required minimum AIX build release. This
> > needs to be doublechecked when attempting the cleanup, though.
> >
> 
> I think  this should be done in a separate change .
> Can we assume  minimum AIX 7.1  at  build+runtime  (I guess this would
> make things easier and it is anyway documented in the build platforms Wiki)
> ?
> 
> Currently I see these usages of  libperfstat::
> 
> os_aix.cpp
> 
> 748  const int rc = libperfstat::perfstat_memory_total(NULL, &psmt,
> sizeof(psmt), 1);
> 1400 libperfstat::wparinfo_t wi;
> 1401 if (libperfstat::get_wparinfo(&wi)) {
> 1409 libperfstat::partitioninfo_t pi;
> 1410 if (libperfstat::get_partitioninfo(&pi)) {
> 3482 libperfstat::perfstat_reset();
> 4004 // AIX: use libperfstat
> 4005 libperfstat::cpuinfo_t ci;
> 4006 if (libperfstat::get_cpuinfo(&ci)) {
> 4183 if (!libperfstat::init()) {
> 
> 
> Best regards, Matthias
> 
> > -----Original Message-----
> > From: Langer, Christoph
> > Sent: Montag, 15. April 2019 08:13
> > To: Baesken, Matthias <matthias.baesken at sap.com>; ppc-aix-port-
> > dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net' <hotspot-
> > dev at openjdk.java.net>
> > Subject: RE: RFR : 8222280 : Provide virtualization related info in the
> hs_error
> > file on AIX
> >
> > Hi Matthias,
> >
> > I think the change is good.
> >
> > As regards to libperfstat usage: I think we should clean this up and try to
> link
> > during the build/use the system headers. We don't need to support AIX
> V5.3
> > as build release any longer and probably all required functionality is
> > contained in the headers of the required minimum AIX build release. This
> > needs to be doublechecked when attempting the cleanup, though.
> >
> > Best regards
> > Christoph
> >
> > > -----Original Message-----
> > > From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf
> > Of
> > > Baesken, Matthias
> > > Sent: Freitag, 12. April 2019 14:52
> > > To: ppc-aix-port-dev at openjdk.java.net; 'hotspot-
> dev at openjdk.java.net'
> > > <hotspot-dev at openjdk.java.net>
> > > Subject: RFR : 8222280 : Provide virtualization related info in the hs_error
> > file
> > > on AIX
> > >
> > > Hello, please review the following  change . It brings  AIX related
> > > virtualization information into the hs_err file .
> > >
> > > While preparing the patch  I wondered a bit  about the   libperfstat   usage
> > in
> > > OpenJDK  (Hotspot  compared to JDK ).
> > > In JDK  we link already to  libperfstat at build time , see  :
> > >
> > > jdk/make/lib/Lib-java.management.gmk
> > > BUILD_LIBMANAGEMENT
> > >
> > > jdk/make/lib/Lib-jdk.management.gmk
> > > BUILD_LIBMANAGEMENT_EXT
> > >
> > >
> > > While in hotspot dlopen/dladdr is used ,  see
> > > jdk/src/hotspot/os/aix/libperfstat_aix.cpp
> > >
> > > 58 bool libperfstat::init() {
> > > 59
> > > 60  // Dynamically load the libperfstat porting library.
> > > 61  g_libhandle = dlopen("/usr/lib/libperfstat.a(shr_64.o)",
> RTLD_MEMBER
> > |
> > > RTLD_NOW);
> > > 62  if (!g_libhandle) {
> > > 63    trcVerbose("Cannot load libperfstat.a (dlerror: %s)", dlerror());
> > > 64    return false;
> > > 65  }
> > >
> > > we  dlopen  and then resolve symbols at runtime.
> > > This was probably a good thing back then when AIX 5.3 was still
> supported
> > as
> > > build platform  ,  but I am not sure if it is still needed these days  on AIX
> > > 7.1/7.2  .
> > >
> > > But  in case  the  dlopen/dladdr approach  in HS  is still needed/wanted , I
> > > would  switch  to  the   libperfstat::   functions .
> > >
> > > Bug/webrev  :
> > >
> > > https://bugs.openjdk.java.net/browse/JDK-8222280
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8222280_aix_virt.0/
> > >
> > >
> > > Thanks , Matthias