Re: PCP patch for top/libgtop conversion

[Mike Mason <mmlnx@xxxxxxxxxx>]

Mark,

Sorry for missing some of the help text.  I thought I'd taken care of 
that :-)

See comments below:

Mark Goodwin wrote:

> > (modified) Per process command name
> >      proc.psinfo.cmd
> > (new) Per process command line from /proc/<pid>/cmdline
> >      proc.psinfo.psargs
>
>
> OK, though not sure if they are needed since the instance name for
> each value already gives this info.

First, I got these backwards:  proc.psinfo.cmd is new and 
proc.psinfo.psargs is modified.

Top sometimes needs the full command line.  Until this patch, neither 
psargs nor the instance name actually gave the full command line.  Now 
they both do.  Top also wants just the command name at times, so I added 
proc.psinfo.cmd.  I could parse the command name out of the instance 
name, but I thought it was easier to use proc.psinfo.cmd, which grabs 
the command name from /proc/<pid>/status.  Either way works for me. 
It's your call.

> > (new) Per process wait channel symbol name
> >      proc.psinfo.wchan_s
>
>
> Looks OK. I assume a zero length string means the processes is not blocked.
> Also, my pmcd log reported :
>
>         Warning: proc.psinfo.wchan_s symbol names may not be accurate!

You should have also seen a warning about an address mismatch between 
System.map and /proc/ksyms.  Did that message appear as well?

Yes, a zero length string means the wait channel address is 0 and the 
process is not blocked.

> > (new) Per process signal info from /proc/<pid>/status
> >      proc.psinfo.signal_s
> >      proc.psinfo.blocked_s
> >      proc.psinfo.sigignore_s
> >      proc.psinfo.sigcatch_s
>
>
> Seems OK, however I'm getting possibly unexpected values for swapped
> processes, e.g. for proc.psinfo.signal_s (pending signals mask) :
>
>     inst [959 or "000959 (nfsd)"] value "fffffffffffffef8"
>
> Did a signed int go negative or something?

I'm just passing through the output from /proc/<pid>/status.  I'll have 
to check what's output when a process is swapped.

> > (new) Per process map info from /proc/<pid>/maps
> >      proc.memory.maps
>
>
> Seems OK. Did you need this to be specifically the same as cat /proc/*/maps
> i.e. a very long string? Does it still work OK if the maps for a particular
> process exceeds the copyout limit (one page)?

This is needed by libgtop.  I couldn't think of any other way to 
represent the maps in PCP.  As far as I know, there's no way to 
represent instances within instances.  Have you ever considered adding 
the capability?

I don't think the copyout limit is an issue for PCP.  There's code in 
proc/array.c to handle maps files greater than 4K.  We just read it, so 
I don't think we need to do anything special.

> > (new) Per process memory info from /proc/<pid>/status
> >      proc.memory.vmsize
> >      proc.memory.vmlock
> >      proc.memory.vmrss
> >      proc.memory.vmdata
> >      proc.memory.vmstack
> >      proc.memory.vmexe
> >      proc.memory.vmlib
>
>
> The values for these metrics all seem to be off by
> one line in /proc/PID/status. e.g. pminfo reports:
>
> proc.memory.vmsize
>     inst [20396 or "020396 -tcsh"] value 0 
> proc.memory.vmlock
>     inst [20396 or "020396 -tcsh"] value 1528
> proc.memory.vmrss
>     inst [20396 or "020396 -tcsh"] value 756
> proc.memory.vmdata
>     inst [20396 or "020396 -tcsh"] value 172
> proc.memory.vmstack
>     inst [20396 or "020396 -tcsh"] value 268
> proc.memory.vmexe
>     inst [20396 or "020396 -tcsh"] value 1456
> proc.memory.vmlib: pmLookupDesc: Unknown or illegal metric identifier
>
> but /proc/20396/status reports:
>
> # cat /proc/20396/status | grep  '^Vm'
> VmSize:     2804 kB
> VmLck:         0 kB
> VmRSS:      1528 kB
> VmData:      756 kB
> VmStk:       172 kB
> VmExe:       268 kB
> VmLib:      1456 kB
>
> The fix is probably trivial ...

I'll look into this.  What version of Linux are you using?

Regards,
Mike

--
Mike Mason
IBM Linux Technology Center, RAS Group
Beaverton, OR, USA
mmlnx@xxxxxxxxxx
(503) 578-4123