From 4a59d4e9b764b52ce3936d5bb5fddee15c554d3e Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 29 May 2014 14:50:36 +0100 Subject: [PATCH] Fix problems identified by rb in review . Copied from Perforce Change: 186347 ServerID: perforce.ravenbrook.com --- mps/code/arenavm.c | 24 ++++++++++++++--------- mps/code/eventdef.h | 7 ++++--- mps/code/gcbench.c | 22 +++++++++++++++++++++- mps/code/mpm.h | 3 ++- mps/code/vman.c | 46 +++++++++++++++++++++++++-------------------- mps/code/vmix.c | 31 ++++++++++++++++++------------ mps/code/vmw3.c | 15 ++++++++++----- 7 files changed, 97 insertions(+), 51 deletions(-) diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c index f878092bd01..e738ffaefa2 100644 --- a/mps/code/arenavm.c +++ b/mps/code/arenavm.c @@ -272,8 +272,9 @@ static void vmArenaUnmap(VMArena vmArena, VM vm, Addr base, Addr limit) * chunkReturn, return parameter for the created chunk. * vmArena, the parent VMArena. * size, approximate amount of virtual address that the chunk should reserve. + * align, minimum acceptable tract size */ -static Res VMChunkCreate(Chunk *chunkReturn, VMArena vmArena, Size size) +static Res VMChunkCreate(Chunk *chunkReturn, VMArena vmArena, Size size, Align align) { Res res; Addr base, limit, chunkStructLimit; @@ -288,12 +289,12 @@ static Res VMChunkCreate(Chunk *chunkReturn, VMArena vmArena, Size size) AVERT(VMArena, vmArena); AVER(size > 0); - res = VMCreate(&vm, size, vmArena->vmParams); + res = VMCreate(&vm, size, align, vmArena->vmParams); if (res != ResOK) goto failVMCreate; + /* The VM will have adjusted align and size; pick up the actual values. */ pageSize = VMAlign(vm); - /* The VM will have aligned the userSize; pick up the actual size. */ base = VMBase(vm); limit = VMLimit(vm); @@ -483,6 +484,7 @@ ARG_DEFINE_KEY(arena_contracted, Fun); static Res VMArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) { Size userSize = VM_ARENA_SIZE_DEFAULT; /* size requested by user */ + Align userAlign = MPS_PF_ALIGN; /* alignment requested by user */ Size chunkSize; /* size actually created */ Size vmArenaSize; /* aligned size of VMArenaStruct */ Res res; @@ -499,19 +501,22 @@ static Res VMArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) if (ArgPick(&arg, args, MPS_KEY_ARENA_SIZE)) userSize = arg.val.size; + if (ArgPick(&arg, args, MPS_KEY_ALIGN)) + userAlign = AlignAlignUp(arg.val.align, userAlign); AVER(userSize > 0); + AVERT(Align, userAlign); - /* Parse the arguments into VM parameters, if any. We must do this into - some stack-allocated memory for the moment, since we don't have anywhere - else to put it. It gets copied later. */ + /* Parse remaining arguments, if any, into VM parameters. We must do + this into some stack-allocated memory for the moment, since we + don't have anywhere else to put it. It gets copied later. */ res = VMParamFromArgs(vmParams, sizeof(vmParams), args); if (res != ResOK) goto failVMCreate; /* Create a VM to hold the arena and map it. */ vmArenaSize = SizeAlignUp(sizeof(VMArenaStruct), MPS_PF_ALIGN); - res = VMCreate(&arenaVM, vmArenaSize, vmParams); + res = VMCreate(&arenaVM, vmArenaSize, userAlign, vmParams); if (res != ResOK) goto failVMCreate; res = VMMap(arenaVM, VMBase(arenaVM), VMLimit(arenaVM)); @@ -548,7 +553,7 @@ static Res VMArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) /* have to have a valid arena before calling ChunkCreate */ vmArena->sig = VMArenaSig; - res = VMChunkCreate(&chunk, vmArena, userSize); + res = VMChunkCreate(&chunk, vmArena, userSize, userAlign); if (res != ResOK) goto failChunkCreate; @@ -714,7 +719,8 @@ static Res VMArenaGrow(Arena arena, SegPref pref, Size size) VMArenaReserved(VMArena2Arena(vmArena))); return res; } - res = VMChunkCreate(&newChunk, vmArena, chunkSize); + res = VMChunkCreate(&newChunk, vmArena, chunkSize, + VMAlign(vmArena->vm)); if(res == ResOK) goto vmArenaGrow_Done; } diff --git a/mps/code/eventdef.h b/mps/code/eventdef.h index c4918d38413..44de7046ed2 100644 --- a/mps/code/eventdef.h +++ b/mps/code/eventdef.h @@ -36,7 +36,7 @@ */ #define EVENT_VERSION_MAJOR ((unsigned)1) -#define EVENT_VERSION_MEDIAN ((unsigned)2) +#define EVENT_VERSION_MEDIAN ((unsigned)3) #define EVENT_VERSION_MINOR ((unsigned)0) @@ -370,8 +370,9 @@ #define EVENT_VMCreate_PARAMS(PARAM, X) \ PARAM(X, 0, P, vm) \ - PARAM(X, 1, A, base) \ - PARAM(X, 2, A, limit) + PARAM(X, 1, W, align) \ + PARAM(X, 2, A, base) \ + PARAM(X, 3, A, limit) #define EVENT_VMDestroy_PARAMS(PARAM, X) \ PARAM(X, 0, P, vm) diff --git a/mps/code/gcbench.c b/mps/code/gcbench.c index 733dc0a925b..f931fe72f6b 100644 --- a/mps/code/gcbench.c +++ b/mps/code/gcbench.c @@ -47,6 +47,7 @@ static double pupdate = 0.1; /* probability of update */ static unsigned ngen = 0; /* number of generations specified */ static mps_gen_param_s gen[genLIMIT]; /* generation parameters */ static size_t arenasize = 256ul * 1024 * 1024; /* arena size */ +static mps_align_t arena_align = 1; /* arena alignment */ static unsigned pinleaf = FALSE; /* are leaf objects pinned at start */ static mps_bool_t zoned = TRUE; /* arena allocates using zones */ @@ -228,6 +229,7 @@ static void arena_setup(gcthread_fn_t fn, { MPS_ARGS_BEGIN(args) { MPS_ARGS_ADD(args, MPS_KEY_ARENA_SIZE, arenasize); + MPS_ARGS_ADD(args, MPS_KEY_ALIGN, arena_align); MPS_ARGS_ADD(args, MPS_KEY_ARENA_ZONED, zoned); RESMUST(mps_arena_create_k(&arena, mps_arena_class_vm(), args)); } MPS_ARGS_END(args); @@ -263,6 +265,7 @@ static struct option longopts[] = { {"npass", required_argument, NULL, 'p'}, {"gen", required_argument, NULL, 'g'}, {"arena-size",required_argument, NULL, 'm'}, + {"arena-align",required_argument, NULL, 'a'}, {"width", required_argument, NULL, 'w'}, {"depth", required_argument, NULL, 'd'}, {"preuse", required_argument, NULL, 'r'}, @@ -299,7 +302,7 @@ int main(int argc, char *argv[]) { } putchar('\n'); - while ((ch = getopt_long(argc, argv, "ht:i:p:g:m:w:d:r:u:lx:z", longopts, NULL)) != -1) + while ((ch = getopt_long(argc, argv, "ht:i:p:g:m:a:w:d:r:u:lx:z", longopts, NULL)) != -1) switch (ch) { case 't': nthreads = (unsigned)strtoul(optarg, NULL, 10); @@ -353,6 +356,20 @@ int main(int argc, char *argv[]) { } } break; + case 'a': { + char *p; + arena_align = (unsigned)strtoul(optarg, &p, 10); + switch(toupper(*p)) { + case 'G': arena_align <<= 30; break; + case 'M': arena_align <<= 20; break; + case 'K': arena_align <<= 10; break; + case '\0': break; + default: + fprintf(stderr, "Bad arena alignment %s\n", optarg); + return EXIT_FAILURE; + } + } + break; case 'w': width = (size_t)strtoul(optarg, NULL, 10); break; @@ -389,6 +406,8 @@ int main(int argc, char *argv[]) { " Use multiple times for multiple generations.\n" " -m n, --arena-size=n[KMG]?\n" " Initial size of arena (default %lu).\n" + " -a n, --arena-align=n[KMG]?\n" + " Alignment of arena (default %lu).\n" " -w n, --width=n\n" " Width of tree nodes made (default %lu)\n", argv[0], @@ -396,6 +415,7 @@ int main(int argc, char *argv[]) { niter, npass, (unsigned long)arenasize, + (unsigned long)arena_align, (unsigned long)width); fprintf(stderr, " -d n, --depth=n\n" diff --git a/mps/code/mpm.h b/mps/code/mpm.h index e6a0da4e834..5162a54abf1 100644 --- a/mps/code/mpm.h +++ b/mps/code/mpm.h @@ -98,6 +98,7 @@ extern Addr (AddrAlignDown)(Addr addr, Align align); #define IndexAlignUp(s, a) ((Index)WordAlignUp((Word)(s), a)) #define IndexAlignDown(s, a) ((Index)WordAlignDown((Word)(s), a)) +#define AlignAlignUp(a1, a2) ((Align)WordAlignUp((Word)(a1), a2)) #define AlignIsAligned(a1, a2) WordIsAligned((Word)(a1), a2) @@ -992,7 +993,7 @@ extern Res RootsIterate(Globals arena, RootIterateFn f, void *p); extern Align VMAlign(VM vm); extern Bool VMCheck(VM vm); extern Res VMParamFromArgs(void *params, size_t paramSize, ArgList args); -extern Res VMCreate(VM *VMReturn, Size size, void *params); +extern Res VMCreate(VM *VMReturn, Size size, Align align, void *params); extern void VMDestroy(VM vm); extern Addr VMBase(VM vm); extern Addr VMLimit(VM vm); diff --git a/mps/code/vman.c b/mps/code/vman.c index 2e68118e27d..5edd9dd5d19 100644 --- a/mps/code/vman.c +++ b/mps/code/vman.c @@ -19,7 +19,8 @@ SRCID(vman, "$Id$"); /* ANSI fake VM structure, see */ typedef struct VMStruct { Sig sig; /* */ - Addr base, limit; /* boundaries of malloc'd memory */ + Align align; /* page size */ + Addr base, limit; /* aligned boundaries of malloc'd memory */ void *block; /* pointer to malloc'd block, for free() */ Size reserved; /* total reserved address space */ Size mapped; /* total mapped memory */ @@ -31,11 +32,12 @@ typedef struct VMStruct { Bool VMCheck(VM vm) { CHECKS(VM, vm); + CHECKD_NOSIG(Align, vm->align); CHECKL(vm->base != (Addr)0); CHECKL(vm->limit != (Addr)0); CHECKL(vm->base < vm->limit); - CHECKL(AddrIsAligned(vm->base, VMANPageALIGNMENT)); - CHECKL(AddrIsAligned(vm->limit, VMANPageALIGNMENT)); + CHECKL(AddrIsAligned(vm->base, vm->align)); + CHECKL(AddrIsAligned(vm->limit, vm->align)); CHECKL(vm->block != NULL); CHECKL((Addr)vm->block <= vm->base); CHECKL(vm->mapped <= vm->reserved); @@ -47,8 +49,8 @@ Bool VMCheck(VM vm) Align VMAlign(VM vm) { - UNUSED(vm); - return VMANPageALIGNMENT; + AVERT(VM, vm); + return vm->align; } @@ -63,19 +65,22 @@ Res VMParamFromArgs(void *params, size_t paramSize, ArgList args) /* VMCreate -- reserve some virtual address space, and create a VM structure */ -Res VMCreate(VM *vmReturn, Size size, void *params) +Res VMCreate(VM *vmReturn, Size size, Align align, void *params) { VM vm; AVER(vmReturn != NULL); + AVER(size > 0); + AVERT(Align, align); AVER(params != NULL); + + align = AlignAlignUp(align, VMANPageALIGNMENT); - /* Note that because we add VMANPageALIGNMENT rather than */ - /* VMANPageALIGNMENT-1 we are not in danger of overflowing */ - /* vm->limit even if malloc were perverse enough to give us */ - /* a block at the end of memory. */ - size = SizeAlignUp(size, VMANPageALIGNMENT) + VMANPageALIGNMENT; - if ((size < VMANPageALIGNMENT) || (size > (Size)(size_t)-1)) + /* Note that because we add align rather than align-1 we are not in + * danger of overflowing vm->limit even if malloc were perverse + * enough to give us a block at the end of memory. */ + size = SizeAlignUp(size, align) + align; + if ((size < align) || (size > (Size)(size_t)-1)) return ResRESOURCE; vm = (VM)malloc(sizeof(VMStruct)); @@ -88,22 +93,23 @@ Res VMCreate(VM *vmReturn, Size size, void *params) return ResMEMORY; } - vm->base = AddrAlignUp((Addr)vm->block, VMANPageALIGNMENT); - vm->limit = AddrAdd(vm->base, size - VMANPageALIGNMENT); + vm->align = align; + vm->base = AddrAlignUp((Addr)vm->block, align); + vm->limit = AddrAdd(vm->base, size - align); AVER(vm->limit < AddrAdd((Addr)vm->block, size)); memset((void *)vm->block, VMJunkBYTE, size); /* Lie about the reserved address space, to simulate real */ /* virtual memory. */ - vm->reserved = size - VMANPageALIGNMENT; + vm->reserved = size - align; vm->mapped = (Size)0; vm->sig = VMSig; AVERT(VM, vm); - EVENT3(VMCreate, vm, vm->base, vm->limit); + EVENT4(VMCreate, vm, vm->align, vm->base, vm->limit); *vmReturn = vm; return ResOK; } @@ -178,8 +184,8 @@ Res VMMap(VM vm, Addr base, Addr limit) AVER(vm->base <= base); AVER(base < limit); AVER(limit <= vm->limit); - AVER(AddrIsAligned(base, VMANPageALIGNMENT)); - AVER(AddrIsAligned(limit, VMANPageALIGNMENT)); + AVER(AddrIsAligned(base, vm->align)); + AVER(AddrIsAligned(limit, vm->align)); size = AddrOffset(base, limit); memset((void *)base, (int)0, size); @@ -201,8 +207,8 @@ void VMUnmap(VM vm, Addr base, Addr limit) AVER(vm->base <= base); AVER(base < limit); AVER(limit <= vm->limit); - AVER(AddrIsAligned(base, VMANPageALIGNMENT)); - AVER(AddrIsAligned(limit, VMANPageALIGNMENT)); + AVER(AddrIsAligned(base, vm->align)); + AVER(AddrIsAligned(limit, vm->align)); size = AddrOffset(base, limit); memset((void *)base, 0xCD, size); diff --git a/mps/code/vmix.c b/mps/code/vmix.c index 0903a031a2c..f4ca1e1a481 100644 --- a/mps/code/vmix.c +++ b/mps/code/vmix.c @@ -65,7 +65,9 @@ SRCID(vmix, "$Id$"); typedef struct VMStruct { Sig sig; /* */ Align align; /* page size */ - Addr base, limit; /* boundaries of reserved space */ + void *mapped_base; /* unaligned base of mmap'd memory */ + size_t mapped_size; /* size of mmap'd memory */ + Addr base, limit; /* aligned boundaries of reserved space */ Size reserved; /* total reserved address space */ Size mapped; /* total mapped memory */ } VMStruct; @@ -106,11 +108,12 @@ Res VMParamFromArgs(void *params, size_t paramSize, ArgList args) /* VMCreate -- reserve some virtual address space, and create a VM structure */ -Res VMCreate(VM *vmReturn, Size size, void *params) +Res VMCreate(VM *vmReturn, Size size, Align align, void *params) { - Align align; VM vm; int pagesize; + size_t mapped_size; + Align pagealign; void *addr; Res res; @@ -119,18 +122,21 @@ Res VMCreate(VM *vmReturn, Size size, void *params) /* Find out the page size from the OS */ pagesize = getpagesize(); - /* check the actual returned pagesize will fit in an object of */ - /* type Align. */ + + /* Check the actual returned pagesize will fit in an object of type + * Align, and that it is a valid alignment. */ AVER(pagesize > 0); AVER((unsigned long)pagesize <= (unsigned long)(Align)-1); - align = (Align)pagesize; - AVER(SizeIsP2(align)); + pagealign = (Align)pagesize; + AVERT(Align, pagealign); + + align = AlignAlignUp(align, pagealign); size = SizeAlignUp(size, align); if((size == 0) || (size > (Size)(size_t)-1)) return ResRESOURCE; - /* Map in a page to store the descriptor on. */ - addr = mmap(0, (size_t)SizeAlignUp(sizeof(VMStruct), align), + /* Map in some pages to store the descriptor on. */ + addr = mmap(0, (size_t)SizeAlignUp(sizeof(VMStruct), pagealign), PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); @@ -147,7 +153,7 @@ Res VMCreate(VM *vmReturn, Size size, void *params) vm->align = align; /* See .assume.not-last. */ - addr = mmap(0, (size_t)size, + addr = mmap(0, (size_t)(size + align - pagealign), PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0); if(addr == MAP_FAILED) { @@ -157,7 +163,8 @@ Res VMCreate(VM *vmReturn, Size size, void *params) goto failReserve; } - vm->base = (Addr)addr; + vm->mapped_base = addr; + vm->base = AddrAlignUp(addr, align); vm->limit = AddrAdd(vm->base, size); vm->reserved = size; vm->mapped = (Size)0; @@ -166,7 +173,7 @@ Res VMCreate(VM *vmReturn, Size size, void *params) AVERT(VM, vm); - EVENT3(VMCreate, vm, vm->base, vm->limit); + EVENT4(VMCreate, vm, vm->align, vm->base, vm->limit); *vmReturn = vm; return ResOK; diff --git a/mps/code/vmw3.c b/mps/code/vmw3.c index 5be8153c73c..9d8d2d57008 100644 --- a/mps/code/vmw3.c +++ b/mps/code/vmw3.c @@ -116,7 +116,7 @@ Res VMParamFromArgs(void *params, size_t paramSize, ArgList args) /* VMCreate -- reserve some virtual address space, and create a VM structure */ -Res VMCreate(VM *vmReturn, Size size, void *params) +Res VMCreate(VM *vmReturn, Size size, Align align, void *params) { LPVOID vbase; SYSTEM_INFO si; @@ -133,9 +133,14 @@ Res VMCreate(VM *vmReturn, Size size, void *params) AVER(COMPATTYPE(SIZE_T, Size)); GetSystemInfo(&si); - align = (Align)si.dwPageSize; - AVER((DWORD)align == si.dwPageSize); /* check it didn't truncate */ - AVER(SizeIsP2(align)); /* see .assume.sysalign */ + + /* Check that the page size will fit in an object of type Align, and + * that it is a valid alignment (see .assume.sysalign). */ + AVER(si.dwPageSize > 0); + AVER(si.dwPageSize <= (DWORD)(Align)-1); + AVERT(Align, (Align)si.dwPageSize); + + align = AlignAlignUp(align, (Align)si.dwPageSize); size = SizeAlignUp(size, align); if ((size == 0) || (size > (Size)(SIZE_T)-1)) return ResRESOURCE; @@ -171,7 +176,7 @@ Res VMCreate(VM *vmReturn, Size size, void *params) vm->sig = VMSig; AVERT(VM, vm); - EVENT3(VMCreate, vm, vm->base, vm->limit); + EVENT4(VMCreate, vm, vm->align, vm->base, vm->limit); *vmReturn = vm; return ResOK;