This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[COMMITTED] Use ALIGN_DOWN in systrim.


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.

Committed.

2015-09-14  Carlos O'Donell  <carlos@redhat.com>
 
	* malloc/malloc.c (systrim): Use ALIGN_DOWN.

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 452f036..0eca9ce 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -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;
---

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]