public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* memory reported in /proc/pid/status is wrongly scaled
@ 2018-08-17 15:35 Livio Bertacco
  2018-08-17 18:44 ` Corinna Vinschen
  0 siblings, 1 reply; 5+ messages in thread
From: Livio Bertacco @ 2018-08-17 15:35 UTC (permalink / raw)
  To: cygwin

 Hi,
While playing with reading process memory usage in Linux and cygwin, I
found that cygwin reports too large values in /proc/self/status (in 2.10.0
and earlier).
Whenever I was allocating a few kB in my test program, the VmSize line in
/proc/self/status was growing several times faster.

Small bash script to show the issue:
#!/bin/bash
pid=$$
vmsizesplit=($(grep VmSize /proc/$pid/status))
vmsize1="${vmsizesplit[1]}"
echo Initial memory reported in status: $vmsize1 kB
echo Allocating a 1000 kB string (bash can use more memory)
eat=$(printf '%1024000s')
vmsizesplit=($(grep VmSize /proc/$pid/status))
vmsize2="${vmsizesplit[1]}"
echo Current memory reported in status: $vmsize2 kB
echo Difference is $[$vmsize2-$vmsize1] kB

Running this in cygwin on my laptop I get:
Initial memory reported in status: 84928 kB
Allocating a 1000 kB string (bash can use more memory)
Current memory reported in status: 106880 kB
Difference is 21952 kB

While bash may use quite more than 1000 kb in this case, 22x times larger
doesn't seem right.

Checking source file fhandler_process.cc, the
function format_process_status which writes the "status" proc file
retrieves memory usage via get_mem_values. Get_mem_values obtains that info
from NtQueryInformationProcess/PagefileUsage which is in bytes, then it
scales it to pages dividing by wincap.page_size:
1515: *vmsize = vmc.PagefileUsage / wincap.page_size ();

Then format_process_status scales it back, in theory to bytes, and shifts
it by 10 bits in order to print it out in kB:
1219:  unsigned page_size = wincap.allocation_granularity ();
1220:  vmsize *= page_size;

The first observation is that scaling and unscaling are using different
factors, causing the issue. Same for the other memory amounts reported in
the "status" file (VmLck, VmRSS, etc....). Memory reported in the "stat"
file seems ok, and regarding the "statm" file, I'm not really sure (since
I'm not sure about what the correct page size should be in cygwin).
A second observation is that the same struct that contains PagefileUsage
also contains PeakPagefileUsage, so it would be very easy to also add the
VmPeak line to the "status" file (as in Linux).

Regards,
Livio

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: memory reported in /proc/pid/status is wrongly scaled
  2018-08-17 15:35 memory reported in /proc/pid/status is wrongly scaled Livio Bertacco
@ 2018-08-17 18:44 ` Corinna Vinschen
  2018-08-17 20:36   ` Corinna Vinschen
  0 siblings, 1 reply; 5+ messages in thread
From: Corinna Vinschen @ 2018-08-17 18:44 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

On Aug 17 16:05, Livio Bertacco wrote:
>  Hi,
> While playing with reading process memory usage in Linux and cygwin, I
> found that cygwin reports too large values in /proc/self/status (in 2.10.0
> and earlier).
> Whenever I was allocating a few kB in my test program, the VmSize line in
> /proc/self/status was growing several times faster.
> 
> Small bash script to show the issue:
> #!/bin/bash
> pid=$$
> vmsizesplit=($(grep VmSize /proc/$pid/status))
> vmsize1="${vmsizesplit[1]}"
> echo Initial memory reported in status: $vmsize1 kB
> echo Allocating a 1000 kB string (bash can use more memory)
> eat=$(printf '%1024000s')
> vmsizesplit=($(grep VmSize /proc/$pid/status))
> vmsize2="${vmsizesplit[1]}"
> echo Current memory reported in status: $vmsize2 kB
> echo Difference is $[$vmsize2-$vmsize1] kB
> 
> Running this in cygwin on my laptop I get:
> Initial memory reported in status: 84928 kB
> Allocating a 1000 kB string (bash can use more memory)
> Current memory reported in status: 106880 kB
> Difference is 21952 kB
> 
> While bash may use quite more than 1000 kb in this case, 22x times larger
> doesn't seem right.
> 
> Checking source file fhandler_process.cc, the
> function format_process_status which writes the "status" proc file
> retrieves memory usage via get_mem_values. Get_mem_values obtains that info
> from NtQueryInformationProcess/PagefileUsage which is in bytes, then it
> scales it to pages dividing by wincap.page_size:
> 1515: *vmsize = vmc.PagefileUsage / wincap.page_size ();
> 
> Then format_process_status scales it back, in theory to bytes, and shifts
> it by 10 bits in order to print it out in kB:
> 1219:  unsigned page_size = wincap.allocation_granularity ();

Looks like this is the bug.  get_mem_values returns all values
in multiple of OS page_size (4K), but format_process_status multiplies
with allocation_granularity (64K), leading to 16 times overallocation
value.  The other caller of get_mem_values, format_process_statm,
returns number of pages.  This must be expressed in multiples of
allocation_granularity since that's the virtual page_size in Cygwin.
But in case of format_process_status we're looking at KB values, so
patch 8a32c24a7bdb0, replaceing page_size with allocation_granularity,
was incorrect.

Good catch!

I'll revert patch 8a32c24a7bdb0 for 2.11.0.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: memory reported in /proc/pid/status is wrongly scaled
  2018-08-17 18:44 ` Corinna Vinschen
@ 2018-08-17 20:36   ` Corinna Vinschen
  0 siblings, 0 replies; 5+ messages in thread
From: Corinna Vinschen @ 2018-08-17 20:36 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 3103 bytes --]

On Aug 17 19:14, Corinna Vinschen wrote:
> On Aug 17 16:05, Livio Bertacco wrote:
> >  Hi,
> > While playing with reading process memory usage in Linux and cygwin, I
> > found that cygwin reports too large values in /proc/self/status (in 2.10.0
> > and earlier).
> > Whenever I was allocating a few kB in my test program, the VmSize line in
> > /proc/self/status was growing several times faster.
> > 
> > Small bash script to show the issue:
> > #!/bin/bash
> > pid=$$
> > vmsizesplit=($(grep VmSize /proc/$pid/status))
> > vmsize1="${vmsizesplit[1]}"
> > echo Initial memory reported in status: $vmsize1 kB
> > echo Allocating a 1000 kB string (bash can use more memory)
> > eat=$(printf '%1024000s')
> > vmsizesplit=($(grep VmSize /proc/$pid/status))
> > vmsize2="${vmsizesplit[1]}"
> > echo Current memory reported in status: $vmsize2 kB
> > echo Difference is $[$vmsize2-$vmsize1] kB
> > 
> > Running this in cygwin on my laptop I get:
> > Initial memory reported in status: 84928 kB
> > Allocating a 1000 kB string (bash can use more memory)
> > Current memory reported in status: 106880 kB
> > Difference is 21952 kB
> > 
> > While bash may use quite more than 1000 kb in this case, 22x times larger
> > doesn't seem right.
> > 
> > Checking source file fhandler_process.cc, the
> > function format_process_status which writes the "status" proc file
> > retrieves memory usage via get_mem_values. Get_mem_values obtains that info
> > from NtQueryInformationProcess/PagefileUsage which is in bytes, then it
> > scales it to pages dividing by wincap.page_size:
> > 1515: *vmsize = vmc.PagefileUsage / wincap.page_size ();
> > 
> > Then format_process_status scales it back, in theory to bytes, and shifts
> > it by 10 bits in order to print it out in kB:
> > 1219:  unsigned page_size = wincap.allocation_granularity ();
> 
> Looks like this is the bug.  get_mem_values returns all values
> in multiple of OS page_size (4K), but format_process_status multiplies
> with allocation_granularity (64K), leading to 16 times overallocation
> value.  The other caller of get_mem_values, format_process_statm,
> returns number of pages.  This must be expressed in multiples of
> allocation_granularity since that's the virtual page_size in Cygwin.
> But in case of format_process_status we're looking at KB values, so
> patch 8a32c24a7bdb0, replaceing page_size with allocation_granularity,
> was incorrect.
> 
> Good catch!
> 
> I'll revert patch 8a32c24a7bdb0 for 2.11.0.

On second thought there's more wrong than just that.  Just dividing
by page_size or allocation_granularity results in too small values.
I applied patches to return more correct values and I made sure
the values in status and statm are consistently rounded up to
Cygwin's page size of 64K.

I uploaded new developer snapshots to https://cygwin.com/snapshots
for testing.  Please give them a try.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: memory reported in /proc/pid/status is wrongly scaled
  2018-08-21 20:04 Livio Bertacco
@ 2018-08-21 20:20 ` Corinna Vinschen
  0 siblings, 0 replies; 5+ messages in thread
From: Corinna Vinschen @ 2018-08-21 20:20 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

On Aug 21 16:53, Livio Bertacco wrote:
> Thanks Corinna, I tried the developer snapshot and the VmSize line from
> "status" file looks good now.
> 
> What do you think about adding the VmPeak line too? (I can create the patch
> myself).
> 
> >  On second thought there's more wrong than just that.
> > Just dividing by page_size or allocation_granularity results in too small
> values.
> 
> Yes, now that you mention it, this rounding up is actually still missing
> for the vmrss and vmmaxrss values in the "stat" file (which just divides by
> page_size):
> 1150
> <https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_process.cc#l1150>
> vmrss = vmc.WorkingSetSize / page_size;
> 1151
> <https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_process.cc#l1151>
> vmmaxrss = ql.MaximumWorkingSetSize / page_size;
> and, by the way, is it correct that these two are reported in 4k pages
> instead of 64k pages?
> "statm" reports e.g. VmRss (that's the second value in the statm line) in
> 64k pages so that value doesn't match the same Rss amount (24th value) from
> "stat".

I'd actually be happy to get patches here.  Just have a look at
https://cygwin.com/contrib.html for the 2-clause BSD statement.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: memory reported in /proc/pid/status is wrongly scaled
@ 2018-08-21 20:04 Livio Bertacco
  2018-08-21 20:20 ` Corinna Vinschen
  0 siblings, 1 reply; 5+ messages in thread
From: Livio Bertacco @ 2018-08-21 20:04 UTC (permalink / raw)
  To: cygwin

Thanks Corinna, I tried the developer snapshot and the VmSize line from
"status" file looks good now.

What do you think about adding the VmPeak line too? (I can create the patch
myself).

>  On second thought there's more wrong than just that.
> Just dividing by page_size or allocation_granularity results in too small
values.

Yes, now that you mention it, this rounding up is actually still missing
for the vmrss and vmmaxrss values in the "stat" file (which just divides by
page_size):
1150
<https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_process.cc#l1150>
vmrss = vmc.WorkingSetSize / page_size;
1151
<https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_process.cc#l1151>
vmmaxrss = ql.MaximumWorkingSetSize / page_size;
and, by the way, is it correct that these two are reported in 4k pages
instead of 64k pages?
"statm" reports e.g. VmRss (that's the second value in the statm line) in
64k pages so that value doesn't match the same Rss amount (24th value) from
"stat".

Livio


> ---------- Forwarded message ----------
> From: Corinna Vinschen <corinna-cygwin@cygwin.com>
> To: cygwin@cygwin.com
> Cc:
> Bcc:
> Date: Fri, 17 Aug 2018 20:44:02 +0200
> Subject: Re: memory reported in /proc/pid/status is wrongly scaled
> On Aug 17 19:14, Corinna Vinschen wrote:
> > On Aug 17 16:05, Livio Bertacco wrote:
> > >  Hi,
> > > While playing with reading process memory usage in Linux and cygwin, I
> > > found that cygwin reports too large values in /proc/self/status (in
> 2.10.0
> > > and earlier).
> > > Whenever I was allocating a few kB in my test program, the VmSize line
> in
> > > /proc/self/status was growing several times faster.
> > >
> > > Small bash script to show the issue:
> > > #!/bin/bash
> > > pid=$$
> > > vmsizesplit=($(grep VmSize /proc/$pid/status))
> > > vmsize1="${vmsizesplit[1]}"
> > > echo Initial memory reported in status: $vmsize1 kB
> > > echo Allocating a 1000 kB string (bash can use more memory)
> > > eat=$(printf '%1024000s')
> > > vmsizesplit=($(grep VmSize /proc/$pid/status))
> > > vmsize2="${vmsizesplit[1]}"
> > > echo Current memory reported in status: $vmsize2 kB
> > > echo Difference is $[$vmsize2-$vmsize1] kB
> > >
> > > Running this in cygwin on my laptop I get:
> > > Initial memory reported in status: 84928 kB
> > > Allocating a 1000 kB string (bash can use more memory)
> > > Current memory reported in status: 106880 kB
> > > Difference is 21952 kB
> > >
> > > While bash may use quite more than 1000 kb in this case, 22x times
> larger
> > > doesn't seem right.
> > >
> > > Checking source file fhandler_process.cc, the
> > > function format_process_status which writes the "status" proc file
> > > retrieves memory usage via get_mem_values. Get_mem_values obtains that
> info
> > > from NtQueryInformationProcess/PagefileUsage which is in bytes, then
> it
> > > scales it to pages dividing by wincap.page_size:
> > > 1515: *vmsize = vmc.PagefileUsage / wincap.page_size ();
> > >
> > > Then format_process_status scales it back, in theory to bytes, and
> shifts
> > > it by 10 bits in order to print it out in kB:
> > > 1219:  unsigned page_size = wincap.allocation_granularity ();
> >
> > Looks like this is the bug.  get_mem_values returns all values
> > in multiple of OS page_size (4K), but format_process_status multiplies
> > with allocation_granularity (64K), leading to 16 times overallocation
> > value.  The other caller of get_mem_values, format_process_statm,
> > returns number of pages.  This must be expressed in multiples of
> > allocation_granularity since that's the virtual page_size in Cygwin.
> > But in case of format_process_status we're looking at KB values, so
> > patch 8a32c24a7bdb0, replaceing page_size with allocation_granularity,
> > was incorrect.
> >
> > Good catch!
> >
> > I'll revert patch 8a32c24a7bdb0 for 2.11.0.
>
> On second thought there's more wrong than just that.  Just dividing
> by page_size or allocation_granularity results in too small values.
> I applied patches to return more correct values and I made sure
> the values in status and statm are consistently rounded up to
> Cygwin's page size of 64K.
>
> I uploaded new developer snapshots to https://cygwin.com/snapshots
> for testing.  Please give them a try.
>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen                  Please, send mails regarding Cygwin to
> Cygwin Maintainer                 cygwin AT cygwin DOT com
> Red Hat
>
>

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-08-21 15:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-17 15:35 memory reported in /proc/pid/status is wrongly scaled Livio Bertacco
2018-08-17 18:44 ` Corinna Vinschen
2018-08-17 20:36   ` Corinna Vinschen
2018-08-21 20:04 Livio Bertacco
2018-08-21 20:20 ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).