Use ALIGN_DOWN in systrim.

While doing code review I converted another bespoke round down, and
corrected a comment.

The comment spoke about keeping at least one page allocated even
during systrim, which is not correct. The code does nothing to keep
a page allocated. The code does attempt to keep PAD padding as
documented in comments and MINSIZE as required by design.

Historically in 2002 when Ulrich wrote the code (fa8d436c) the math
was inlined into one statement which did reserve an extra page:
extra = ((top_size - pad - MINSIZE + (pagesz-1)) / pagesz - 1) * pagesz;
There is no reason given for this extra page.

In 2010 Anton Branchard's change (b9b42ee0) from division
to shifts removed the extra page by dropping the "+ (pagesiz-1), which
mean we might have attempted to return -0 via MORECORE. The fix by Will
Newton in 2014 added a check for extra being zero (51a7380b).

From first principles I see no reason why we should keep an extra
page of memory from being trimmed back to the OS. The only sensible
interface is to honour PAD padding as the function is documented,
with the caveat the MINSIZE is maintained for the top chunk.

Given that we've been using this code for 5+ years with no extra
page allocated is sufficient evidence that the comment should be changed
to match the code that I'm touching.

Tested on x86_64 and i686, no regressions.
This commit is contained in:
Carlos O'Donell 2015-09-14 15:32:47 -04:00
parent 3b2cc56dbc
commit ca6be1655b
2 changed files with 5 additions and 3 deletions

View File

@ -1,5 +1,7 @@
2015-09-14 Carlos O'Donell <carlos@redhat.com>
* malloc/malloc.c (systrim): Use ALIGN_DOWN.
* Makefile ($(objpfx)check-local-headers.out): Redirect stdin from
/dev/null.

View File

@ -241,7 +241,7 @@
/* For MIN, MAX, powerof2. */
#include <sys/param.h>
/* For ALIGN_UP. */
/* For ALIGN_UP et. al. */
#include <libc-internal.h>
@ -2767,8 +2767,8 @@ systrim (size_t pad, mstate av)
if (top_area <= pad)
return 0;
/* Release in pagesize units, keeping at least one page */
extra = (top_area - pad) & ~(pagesize - 1);
/* Release in pagesize units and round down to the nearest page. */
extra = ALIGN_DOWN(top_area - pad, pagesize);
if (extra == 0)
return 0;