Message ID | 87wnbojswh.fsf@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | malloc: Correct the documentation of the top_pad default | expand |
On 2022-08-04 04:48, Florian Weimer via Libc-alpha wrote: > DEFAULT_TOP_PAD is defined as 131072 in > sysdeps/generic/malloc-machine.h. LGTM, but I wonder if we should tighten this so that it doesn't appear that DEFAULT_TOP_PAD could be 0 in any of our build configurations. e.g. we could replace the definition with a #error "define DEFAULT_TOP_PAD" or something like that to ensure that it's always defined through malloc-machine.h. What do you think? Thanks, Sid > > --- > elf/dl-tunables.list | 1 + > elf/tst-rtld-list-tunables.exp | 2 +- > manual/tunables.texi | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list > index e6a56b3070..e925590c36 100644 > --- a/elf/dl-tunables.list > +++ b/elf/dl-tunables.list > @@ -42,6 +42,7 @@ glibc { > type: SIZE_T > env_alias: MALLOC_TOP_PAD_ > security_level: SXID_IGNORE > + default: 131072 > } > perturb { > type: INT_32 > diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp > index cdfdb56a94..2233ea9c7c 100644 > --- a/elf/tst-rtld-list-tunables.exp > +++ b/elf/tst-rtld-list-tunables.exp > @@ -9,7 +9,7 @@ glibc.malloc.perturb: 0 (min: 0, max: 255) > glibc.malloc.tcache_count: 0x0 (min: 0x0, max: 0x[f]+) > glibc.malloc.tcache_max: 0x0 (min: 0x0, max: 0x[f]+) > glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0x[f]+) > -glibc.malloc.top_pad: 0x0 (min: 0x0, max: 0x[f]+) > +glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+) > glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+) > glibc.rtld.dynamic_sort: 2 (min: 1, max: 2) > glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10) > diff --git a/manual/tunables.texi b/manual/tunables.texi > index 83cdcdac6d..ffd50a3fe2 100644 > --- a/manual/tunables.texi > +++ b/manual/tunables.texi > @@ -143,7 +143,7 @@ number of bytes to retain when shrinking any of the arenas. This provides the > necessary hysteresis in heap size such that excessive amounts of system calls > can be avoided. > > -The default value of this tunable is @samp{0}. > +The default value of this tunable is @samp{131072} (128 KB). > @end deftp > > @deftp Tunable glibc.malloc.perturb >
* Siddhesh Poyarekar: > On 2022-08-04 04:48, Florian Weimer via Libc-alpha wrote: >> DEFAULT_TOP_PAD is defined as 131072 in >> sysdeps/generic/malloc-machine.h. > > LGTM, but I wonder if we should tighten this so that it doesn't appear > that DEFAULT_TOP_PAD could be 0 in any of our build configurations. > e.g. we could replace the definition with a #error "define > DEFAULT_TOP_PAD" or something like that to ensure that it's always > defined through malloc-machine.h. What do you think? We could just remove the various #ifdefs, I guess. The other question is whether we can use the constant in the tunable definition file. I haven't investigate any of this. Should I push my change in the meantime? Thanks, Florian
On 2022-08-04 11:14, Florian Weimer wrote: > * Siddhesh Poyarekar: > >> On 2022-08-04 04:48, Florian Weimer via Libc-alpha wrote: >>> DEFAULT_TOP_PAD is defined as 131072 in >>> sysdeps/generic/malloc-machine.h. >> >> LGTM, but I wonder if we should tighten this so that it doesn't appear >> that DEFAULT_TOP_PAD could be 0 in any of our build configurations. >> e.g. we could replace the definition with a #error "define >> DEFAULT_TOP_PAD" or something like that to ensure that it's always >> defined through malloc-machine.h. What do you think? > > We could just remove the various #ifdefs, I guess. The other question > is whether we can use the constant in the tunable definition file. I > haven't investigate any of this. > > Should I push my change in the meantime? Yes please. Thanks, Sid
Hi Florian, On 8/4/22 10:48, Florian Weimer via Libc-alpha wrote: > DEFAULT_TOP_PAD is defined as 131072 in > sysdeps/generic/malloc-machine.h. > > --- > elf/dl-tunables.list | 1 + > elf/tst-rtld-list-tunables.exp | 2 +- > manual/tunables.texi | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > [...] > diff --git a/manual/tunables.texi b/manual/tunables.texi > index 83cdcdac6d..ffd50a3fe2 100644 > --- a/manual/tunables.texi > +++ b/manual/tunables.texi > @@ -143,7 +143,7 @@ number of bytes to retain when shrinking any of the arenas. This provides the > necessary hysteresis in heap size such that excessive amounts of system calls > can be avoided. > > -The default value of this tunable is @samp{0}. > +The default value of this tunable is @samp{131072} (128 KB). I don't know how much glibc uses KB vs KiB, but I'd like to be pedantic here and suggest using KiB (maybe this requires a global change to keep consistency). See units(7). Cheers, Alex > @end deftp > > @deftp Tunable glibc.malloc.perturb >
* Alejandro Colomar: > Hi Florian, > > On 8/4/22 10:48, Florian Weimer via Libc-alpha wrote: >> DEFAULT_TOP_PAD is defined as 131072 in >> sysdeps/generic/malloc-machine.h. >> --- >> elf/dl-tunables.list | 1 + >> elf/tst-rtld-list-tunables.exp | 2 +- >> manual/tunables.texi | 2 +- >> 3 files changed, 3 insertions(+), 2 deletions(-) >> > > [...] > >> diff --git a/manual/tunables.texi b/manual/tunables.texi >> index 83cdcdac6d..ffd50a3fe2 100644 >> --- a/manual/tunables.texi >> +++ b/manual/tunables.texi >> @@ -143,7 +143,7 @@ number of bytes to retain when shrinking any of the arenas. This provides the >> necessary hysteresis in heap size such that excessive amounts of system calls >> can be avoided. >> -The default value of this tunable is @samp{0}. >> +The default value of this tunable is @samp{131072} (128 KB). > > I don't know how much glibc uses KB vs KiB, but I'd like to be > pedantic here and suggest using KiB (maybe this requires a global > change to keep consistency). See units(7). I checked the chapter used KB, but not KiB before this change, so I kept using KB. Maybe we should change all occurrences to KiB. Thanks, Florian
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index e6a56b3070..e925590c36 100644 --- a/elf/dl-tunables.list +++ b/elf/dl-tunables.list @@ -42,6 +42,7 @@ glibc { type: SIZE_T env_alias: MALLOC_TOP_PAD_ security_level: SXID_IGNORE + default: 131072 } perturb { type: INT_32 diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp index cdfdb56a94..2233ea9c7c 100644 --- a/elf/tst-rtld-list-tunables.exp +++ b/elf/tst-rtld-list-tunables.exp @@ -9,7 +9,7 @@ glibc.malloc.perturb: 0 (min: 0, max: 255) glibc.malloc.tcache_count: 0x0 (min: 0x0, max: 0x[f]+) glibc.malloc.tcache_max: 0x0 (min: 0x0, max: 0x[f]+) glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0x[f]+) -glibc.malloc.top_pad: 0x0 (min: 0x0, max: 0x[f]+) +glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+) glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+) glibc.rtld.dynamic_sort: 2 (min: 1, max: 2) glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10) diff --git a/manual/tunables.texi b/manual/tunables.texi index 83cdcdac6d..ffd50a3fe2 100644 --- a/manual/tunables.texi +++ b/manual/tunables.texi @@ -143,7 +143,7 @@ number of bytes to retain when shrinking any of the arenas. This provides the necessary hysteresis in heap size such that excessive amounts of system calls can be avoided. -The default value of this tunable is @samp{0}. +The default value of this tunable is @samp{131072} (128 KB). @end deftp @deftp Tunable glibc.malloc.perturb