[mp2] Apache2::SizeLimit should be using $s->rss, not $s->size forLinux::Smaps

This is a multi-part message in MIME format.
--------------020909050606050102070402
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

All of my processes kept exiting with a report that they had a 300M
unshared size, which was clearly untrue, even from looking at top. After
some investigation, I discovered that Apache2::SizeLimit was calling
$s->size on the Linux::Smaps object, when instead it should be returning
$s->rss as the process size.

Attached is a one-line patch to fix the issue.

Also, if you're interested in proof that this is right, I've attached a
short .cgi file that you can load under mod_perl, with Linux::Smaps
installed, to see process sizes and the return values of all of the
Smaps accessors.

-Max
--
http://www.everythingsolved.com/
Competent, Friendly Bugzilla and Perl Services. Everything Else, too.

--------------020909050606050102070402
Content-Type: text/plain;
name="size-smaps.diff"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
filename="size-smaps.diff"

LS0tIEFwYWNoZTIvU2l6ZUxpbWl0LnBtLm9sZAkyMDEwLTAyLTAyIDE1OjUz OjIyLjAwMDAw
MDAwMCAtMDYwMAorKysgQXBhY2hlMi9TaXplTGltaXQucG0JMjAxMC0wMi0w MiAxNTo1MDox
MS4wMDAwMDAwMDAgLTA2MDAKQEAgLTExMSw3ICsxMTEsNyBAQAogc3ViIGxp bnV4X3NtYXBz
X3NpemVfY2hlY2sgewogCiAgICAgbXkgJHMgPSBMaW51eDo6U21hcHMtPm5l dygkJCktPmFs
bDsKLSAgICByZXR1cm4gKCRzLT5zaXplLCAkcy0+c2hhcmVkX2NsZWFuICsg JHMtPnNoYXJl
ZF9kaXJ0eSk7CisgICAgcmV0dXJuICgkcy0+cnNzLCAkcy0+c2hhcmVkX2Ns ZWFuICsgJHMt
PnNoYXJlZF9kaXJ0eSk7CiB9CiAKICMgcmV0dXJuIHByb2Nlc3Mgc2l6ZSAo aW4gS0IpCg==
--------------020909050606050102070402
Content-Type: text/plain;
name="test.cgi"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
filename="test.cgi"

IyEvdXNyL2Jpbi9wZXJsIC13VAp1c2Ugc3RyaWN0Owp1c2Ugd2FybmluZ3M7 CnVzZSBMaW51
eDo6U21hcHM7CgpteSAkcyA9IExpbnV4OjpTbWFwcy0+bmV3KCQkKS0+YWxs OwpwcmludCAi
Q29udGVudC10eXBlOiB0ZXh0L3BsYWluXG5cbiI7CnByaW50ICJTSVpFOiAi IC4gJHMtPnNp
emUsICJcbiI7CnByaW50ICJDTEVBTjogIiwgJHMtPnNoYXJlZF9jbGVhbiwg IiBESVJUWTog
IiwgJHMtPnNoYXJlZF9kaXJ0eSwgIlxuIjsKcHJpbnQgIlBSSVZBVEUgQ0xF QU46ICIsICRz
LT5wcml2YXRlX2NsZWFuLCAiIFBSSVZBVEUgRElSVFk6ICIsICRzLT5wcml2 YXRlX2RpcnR5
LCAiXG4iOwpwcmludCAiUlNTOiAiLCAkcy0+cnNzLCAiXG4iOwo=
--------------020909050606050102070402--

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->size

Can you submit these patches inline?

http://perl.apache.org/docs/2.0/devel/help/help.html#Submitt ing_Patches

When they are attachments like this, people tend to not read them.
Whereas when they are inline, people tend to be able to read them and
comment very easily.

On Tue, Feb 2, 2010 at 1:58 PM, Max Kanat-Alexander w=
rote:
> =A0 =A0 =A0 =A0All of my processes kept exiting with a report that they h=
ad a 300M
> unshared size, which was clearly untrue, even from looking at top. After
> some investigation, I discovered that Apache2::SizeLimit was calling
> $s->size on the Linux::Smaps object, when instead it should be returning
> $s->rss as the process size.
>
> =A0 =A0 =A0 =A0Attached is a one-line patch to fix the issue.
>
> =A0 =A0 =A0 =A0Also, if you're interested in proof that this is right, I'=
ve attached a
> short .cgi file that you can load under mod_perl, with Linux::Smaps
> installed, to see process sizes and the return values of all of the
> Smaps accessors.
>
> =A0 =A0 =A0 =A0-Max
> --
> http://www.everythingsolved.com/
> Competent, Friendly Bugzilla and Perl Services. Everything Else, too.
>

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->size for Linux::Smaps

On Tuesday 02 February 2010 22:58:13 Max Kanat-Alexander wrote:
> All of my processes kept exiting with a report that they had a 300M
> unshared size, which was clearly untrue, even from looking at top. After
> some investigation, I discovered that Apache2::SizeLimit was calling
> $s->size on the Linux::Smaps object, when instead it should be returning
> $s->rss as the process size.
>
Well, I tend to disagree. (Fred, Adam please read on.)

The /proc/PID/statm based check returns the fields 0 and 2. According to the
following table from KERNEL/Documentation/filesystems/proc.txt field 0 is SIZE
and not RSS.

Table 1-3: Contents of the statm files (as of 2.6.8-rc3)
............................................................ ...................
Field Content
size total program size (pages) (same as VmSize in status)
resident size of memory portions (pages) (same as VmRSS in status)
shared number of pages that are shared (i.e. backed by a file)
trs number of pages that are 'code' (not including libs; broken,
includes data segment)
lrs number of pages of library (always 0 on 2.6)
drs number of pages of data/stack (including libs; broken,
includes library text)
dt number of dirty pages (always 0 on 2.6)

This is also consistent with (Smaps prints kb while statm shows pages, hence
the division by 4):

$ perl -MLinux::Smaps -le 'print Linux::Smaps->new(shift)->size/4' $$
3526
$ cat /proc/$$/statm
3525 881 396 144 0 495 0

So, either the far older statm technique is also wrong or the patch is wrong.

But on Solaris we do "-s /proc/self/as". That is the size of the address space
of the process. Are we wrong there, as well?

Ok, on BSD it seems to be RSS:

# rss is in KB but ixrss is in BYTES.
# This is true on at least FreeBSD, OpenBSD, & NetBSD
sub _bsd_size_check {

my @results = BSD::Resource::getrusage();
my $max_rss = $results[2];
my $max_ixrss = int ( $results[3] / 1024 );

return ($max_rss, $max_ixrss);
}

About the windows code I cannot say anything. What does
"$peak_working_set_size" mean? For me the wording seems a bit similar to
max(resident segment size).

So, we have at least 2 different meanings of the SIZE result. On BSD it is RSS
on Solaris and Linux SIZE. What is correct?

Let's see how it is used?

my ($size, $share, $unshared) = $class->_check_size();

return 1 if $MAX_PROCESS_SIZE && $size > $MAX_PROCESS_SIZE;

return 0 unless $share;

return 1 if $MIN_SHARE_SIZE && $share < $MIN_SHARE_SIZE;

return 1 if $MAX_UNSHARED_SIZE && $unshared > $MAX_UNSHARED_SIZE;

It is compared with $MAX_PROCESS_SIZE and there is no $MAX_PROCESS_RSS.

And what does the docs say?

=item * Apache2::SizeLimit->set_max_process_size($size)

This sets the maximum size of the process, including both shared and
unshared memory.

It talks about process size not RSS, again.

Now let's assume we would check RSS instead of SIZE.

When a new apache worker process is created its growth in SIZE depends on how
much memory it allocates additionally over time. But its RSS depends upon the
process' size and what part of it is swapped out. It seems to me that we want
to kill a worker if its size grows bigger than the initial worker size plus a
certain amount it is allowed to grow. If we would check RSS then a worker or
even all workers could be killed because some administrator does

swapoff /dev/...

and suddenly all pages that were swapped to this device are copied into RAM
and added to the worker's RSS. I think that would be wrong.


But you mentioned "unshared size". How is it calculated?
Apache::SizeLimit::Core::_check_size() simply does $size-$share. On Linux this
is wrong. When you read /proc/PID/smaps the kernel walks through all pages
that belong to the process and does:

size+=pagesize;
if( page_is_in_RAM ) {
rss+=pagesize;
if( reference_count>1 ) { /* how many processes map that page */
shared+=pagesize;
} else {
private+=pagesize;
}
}

When a page is not in RAM there is unfortunately no way to check the reference
count other than swap it in. And that is certainly too high a cost.

Currently Apache2::SizeLimit assumes that everything that is not in core is
not shared. This is certainly wrong.

Perhaps Apache2::SizeLimit::Core should read:

sub _check_size {
my $class = shift;

my ($size, $share, $unshared) = $class->_platform_check_size();

return ($size, $share, defined $unshared ? $unshared : $size - $share);
}

sub _linux_smaps_size_check {
my $class = shift;

return $class->_linux_size_check() unless $USE_SMAPS;

my $s = Linux::Smaps->new($$)->all;
return ($s->size,
$s->shared_clean + $s->shared_dirty,
$s->private_clean + $s->private_dirty);
}

That would be more what you want, I think.

Torsten

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->sizefor Linux::Smaps

On 02/03/2010 04:57 AM, Torsten Förtsch wrote:
> Well, I tend to disagree. (Fred, Adam please read on.)

Okay. Have you looked at the actual output of test.cgi?

Here's an example of these values just on my local machine, for my bash
interpreter:

[mkanat@es-compy ~]$ cat /proc/self/statm
21045 125 107 13 0 65 0
[mkanat@es-compy ~]$ cat /proc/self/status | grep Vm
VmPeak: 84180 kB
VmSize: 84180 kB
VmLck: 0 kB
VmHWM: 496 kB
VmRSS: 496 kB
VmData: 176 kB
VmStk: 84 kB
VmExe: 52 kB
VmLib: 1548 kB
VmPTE: 48 kB

Would you consider that bash is taking up 84MB, or would you consider
it's taking up 496KB? I have 2GB in physical RAM and 2GB in swap. So,
theoretically, I should only be able to open 48 terminals before I OOM.
With 60 terminals open, here's the result of "free":

total used free
Mem: 1990 1855 134
-/+ buffers/cache: 1140 850
Swap: 1999 212 1787

If I were simply to add up the "Virt" column of the top 15 processes in
"top" on my machine, I'd be using somewhere in the vicinity of 20GB of RAM.

> So, either the far older statm technique is also wrong or the patch is wrong.

I would suspect that if you're using column 0 as the size, then statm
is also indeed wrong.

> =item * Apache2::SizeLimit->set_max_process_size($size)
>
> This sets the maximum size of the process, including both shared and
> unshared memory.

Ahh, but look at the output of both the "shared" and "private"
accessors, and they do not add up to VmSize. They add up to RSS.

> Perhaps Apache2::SizeLimit::Core should read:
>
> sub _check_size {
> my $class = shift;
>
> my ($size, $share, $unshared) = $class->_platform_check_size();
>
> return ($size, $share, defined $unshared ? $unshared : $size - $share);

That would certainly help (because then you would get a more accurate
count of the current private RSS), although I'd argue that VmSize is
still meaningless. I can imagine few instances where you'd want to know
the total size of the address space of a process, even if most of it had
never been mapped to an actual memory page.

-Max
--
http://www.everythingsolved.com/
Competent, Friendly Bugzilla and Perl Services. Everything Else, too.

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->sizefor Linux::Smaps

So, I just recently installed the very latest release of SizeLimit, and
this is still a problem. SizeLimit **definitely** returns the wrong
unshared size for processes on any modern Linux system.

-Max

On 02/03/2010 12:05 PM, Max Kanat-Alexander wrote:
> On 02/03/2010 04:57 AM, Torsten Förtsch wrote:
>> Well, I tend to disagree. (Fred, Adam please read on.)
>
> Okay. Have you looked at the actual output of test.cgi?
>
> Here's an example of these values just on my local machine, for my bash
> interpreter:
>
> [mkanat@es-compy ~]$ cat /proc/self/statm
> 21045 125 107 13 0 65 0
> [mkanat@es-compy ~]$ cat /proc/self/status | grep Vm
> VmPeak: 84180 kB
> VmSize: 84180 kB
> VmLck: 0 kB
> VmHWM: 496 kB
> VmRSS: 496 kB
> VmData: 176 kB
> VmStk: 84 kB
> VmExe: 52 kB
> VmLib: 1548 kB
> VmPTE: 48 kB
>
> Would you consider that bash is taking up 84MB, or would you consider
> it's taking up 496KB? I have 2GB in physical RAM and 2GB in swap. So,
> theoretically, I should only be able to open 48 terminals before I OOM.
> With 60 terminals open, here's the result of "free":
>
> total used free
> Mem: 1990 1855 134
> -/+ buffers/cache: 1140 850
> Swap: 1999 212 1787
>
> If I were simply to add up the "Virt" column of the top 15 processes in
> "top" on my machine, I'd be using somewhere in the vicinity of 20GB of RAM.
>
>> So, either the far older statm technique is also wrong or the patch is wrong.
>
> I would suspect that if you're using column 0 as the size, then statm
> is also indeed wrong.
>
>> =item * Apache2::SizeLimit->set_max_process_size($size)
>>
>> This sets the maximum size of the process, including both shared and
>> unshared memory.
>
> Ahh, but look at the output of both the "shared" and "private"
> accessors, and they do not add up to VmSize. They add up to RSS.
>
>> Perhaps Apache2::SizeLimit::Core should read:
>>
>> sub _check_size {
>> my $class = shift;
>>
>> my ($size, $share, $unshared) = $class->_platform_check_size();
>>
>> return ($size, $share, defined $unshared ? $unshared : $size - $share);
>
> That would certainly help (because then you would get a more accurate
> count of the current private RSS), although I'd argue that VmSize is
> still meaningless. I can imagine few instances where you'd want to know
> the total size of the address space of a process, even if most of it had
> never been mapped to an actual memory page.
>
> -Max


--
http://www.bugzillasource.com/
Competent, Friendly Bugzilla, Perl, and IT Services

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->size for Linux::Smaps

On 02/02/2010 02:19 PM, Fred Moyer wrote:
> Can you submit these patches inline?

Here is the patch inline:

Index: lib/Apache/SizeLimit/Core.pm
============================================================ =======
--- lib/Apache/SizeLimit/Core.pm (revision 905815)
+++ lib/Apache/SizeLimit/Core.pm (working copy)
@@ -175,7 +175,7 @@
return $class->_linux_size_check() unless $USE_SMAPS;

my $s = Linux::Smaps->new($$)->all;
- return ($s->size, $s->shared_clean + $s->shared_dirty);
+ return ($s->rss, $s->shared_clean + $s->shared_dirty);
}

sub _linux_size_check {

Re: [mp2] Apache2::SizeLimit should be using $s->rss, not $s->sizefor Linux::Smaps

On Wed, Feb 9, 2011 at 5:24 PM, Max Kanat-Alexander w=
rote:
> On 02/02/2010 02:19 PM, Fred Moyer wrote:
>> Can you submit these patches inline?
>
> =A0 =A0 =A0 =A0Here is the patch inline:

Thanks - expect 0.96 in the next day.

>
> Index: lib/Apache/SizeLimit/Core.pm
> ==================== =====
==================== =====3D=
==================
> --- lib/Apache/SizeLimit/Core.pm =A0 =A0 =A0 =A0(revision 905815)
> +++ lib/Apache/SizeLimit/Core.pm =A0 =A0 =A0 =A0(working copy)
> @@ -175,7 +175,7 @@
> =A0 =A0 return $class->_linux_size_check() unless $USE_SMAPS;
>
> =A0 =A0 my $s =3D Linux::Smaps->new($$)->all;
> - =A0 =A0return ($s->size, $s->shared_clean + $s->shared_dirty);
> + =A0 =A0return ($s->rss, $s->shared_clean + $s->shared_dirty);
> =A0}
>
> =A0sub _linux_size_check {
>