On Fri, May 13, 2022 at 08:21:19PM -0700, Philip Guenther wrote:
> If you try to grow a 'large' (at least half a page) allocation with
> realloc(3), it'll try to extend the memory mapping for it and if that
> works it won't need to move it.
(on a side note: it does not do anything at all if the # of pages does
not grow).
>
> Currently, it does that by using mquery(2) to see if that area is
> available and if so then trying to mmap it, munmaping it if mmap(2) didn't
> return the desired address (perhaps due to a race with another thread).
> Instead of doing mquery/mmap/munmap, this path can use the MAP_FIXED and
> __MAP_NOREPLACE flags to directly request the specific address but not
> change anything if it's not available.
>
>
> (We still use mquery in ld.so on i386 as a performance optimization, but
> that use case differs in needing to find a base address for *multiple*
> mappings, where unrolling partial hits grows to be more expensive than
> probing with mquery and only trying to do all the mapping on a successful
> set of probes: recall that on x86 munmap() is more expensive than mmap()
> due to TLB shootdowns. This case in realloc() only has a single mapping to
> probe/extend so it avoids those costs. Indeed, this diff eliminates the
> current "munmap on failed attempt" path/cost.)
>
>
> The regress/lib/libc/malloc tests continue to pass on my amd64 box, with
> ktrace confirming the change in calls.
>
> oks?
I was looking through the commit log to see when this flag was introduced.
It was in sys/mman.h rev 1.21, but a bit hidden becuase the commit
message talks about __MAP_NOREMAP.
Anyway, I cannot oversee if it is relevant if all pmap implementations
implement this or if this is done in a higher layer.
Also a tiny bit worried the __MAP_NOREPLACE path in uvm isn't
exercised as much, so it might reveal bugs. But thats no reason not to
do this now.
So if my first question can be answered with: it's in a higher layer,
OK.
-Otto
>
>
> Philip
>
>
> Index: stdlib/malloc.c
> ===================================================================
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.273
> diff -u -p -r1.273 malloc.c
> --- stdlib/malloc.c 26 Feb 2022 16:14:42 -0000 1.273
> +++ stdlib/malloc.c 14 May 2022 02:35:04 -0000
> @@ -100,9 +100,6 @@
> #define MMAPA(a,sz,f) mmap((a), (sz), PROT_READ | PROT_WRITE, \
> MAP_ANON | MAP_PRIVATE | (f), -1, 0)
>
> -#define MQUERY(a,sz,f) mquery((a), (sz), PROT_READ | PROT_WRITE, \
> - MAP_ANON | MAP_PRIVATE | MAP_FIXED | (f), -1, 0)
> -
> struct region_info {
> void *p; /* page; low bits used to mark chunks */
> uintptr_t size; /* size for pages, or chunk_info pointer */
> @@ -1687,11 +1684,7 @@ orealloc(struct dir_info **argpool, void
> size_t needed = rnewsz - roldsz;
>
> STATS_INC(pool->cheap_realloc_tries);
> - q = MQUERY(hint, needed, pool->mmap_flag);
> - if (q == hint)
> - q = MMAPA(hint, needed,
> pool->mmap_flag);
> - else
> - q = MAP_FAILED;
> + q = MMAPA(hint, needed, MAP_FIXED |
> __MAP_NOREPLACE | pool->mmap_flag);
> if (q == hint) {
> STATS_ADD(pool->malloc_used, needed);
> if (pool->malloc_junk == 2)
> @@ -1709,9 +1702,6 @@ orealloc(struct dir_info **argpool, void
> STATS_INC(pool->cheap_reallocs);
> ret = p;
> goto done;
> - } else if (q != MAP_FAILED) {
> - if (munmap(q, needed))
> - wrterror(pool, "munmap %p", q);
> }
> }
> } else if (rnewsz < roldsz) {
>
