From 0a9858b956448866bbad108d0aa37605b04b0d30 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 29 Sep 2014 22:37:02 +0100
Subject: [PATCH 01/16] Branching master to branch/2014-09-29/reserved.

Copied from Perforce
 Change: 187086
 ServerID: perforce.ravenbrook.com

From 9436487a051bb5d56ae696c9f6ec4f0c53255cc0 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 29 Sep 2014 22:50:46 +0100
Subject: [PATCH 02/16] Instead of iterating over the chunks to compute the
 total reserved address space, maintain a running total in arenachunkinsert
 and (new function) arenachunkremoved.

New arena class method chunkReserved handles the class-specific computation of the reserved size of a chunk.

Copied from Perforce
 Change: 187089
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c    | 53 ++++++++++++++++++++++++++++++---------------
 mps/code/arenacl.c  | 34 ++++++++++-------------------
 mps/code/arenavm.c  | 42 +++++++++++++++--------------------
 mps/code/mpm.h      |  1 +
 mps/code/mpmst.h    |  5 +++--
 mps/code/mpmtypes.h |  2 +-
 mps/code/tract.c    |  7 +++---
 7 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index 30a65479f9b..e8efe5d9bdb 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -85,13 +85,13 @@ DEFINE_CLASS(AbstractArenaClass, class)
   class->varargs = ArgTrivVarargs;
   class->init = NULL;
   class->finish = NULL;
-  class->reserved = NULL;
   class->purgeSpare = ArenaNoPurgeSpare;
   class->extend = ArenaNoExtend;
   class->grow = ArenaNoGrow;
   class->free = NULL;
   class->chunkInit = NULL;
   class->chunkFinish = NULL;
+  class->chunkReserved = NULL;
   class->compact = ArenaTrivCompact;
   class->describe = ArenaTrivDescribe;
   class->pagesMarkAllocated = NULL;
@@ -113,13 +113,13 @@ Bool ArenaClassCheck(ArenaClass class)
   CHECKL(FUNCHECK(class->varargs));
   CHECKL(FUNCHECK(class->init));
   CHECKL(FUNCHECK(class->finish));
-  CHECKL(FUNCHECK(class->reserved));
   CHECKL(FUNCHECK(class->purgeSpare));
   CHECKL(FUNCHECK(class->extend));
   CHECKL(FUNCHECK(class->grow));
   CHECKL(FUNCHECK(class->free));
   CHECKL(FUNCHECK(class->chunkInit));
   CHECKL(FUNCHECK(class->chunkFinish));
+  CHECKL(FUNCHECK(class->chunkReserved));
   CHECKL(FUNCHECK(class->compact));
   CHECKL(FUNCHECK(class->describe));
   CHECKL(FUNCHECK(class->pagesMarkAllocated));
@@ -206,6 +206,7 @@ Res ArenaInit(Arena arena, ArenaClass class, Size grainSize, ArgList args)
 
   arena->class = class;
 
+  arena->reserved = (Size)0;
   arena->committed = (Size)0;
   /* commitLimit may be overridden by init (but probably not */
   /* as there's not much point) */
@@ -454,7 +455,6 @@ void ControlFinish(Arena arena)
 Res ArenaDescribe(Arena arena, mps_lib_FILE *stream, Count depth)
 {
   Res res;
-  Size reserved;
 
   if (!TESTT(Arena, arena))
     return ResFAIL;
@@ -476,20 +476,9 @@ Res ArenaDescribe(Arena arena, mps_lib_FILE *stream, Count depth)
       return res;
   }
 
-  /* Note: this Describe clause calls a function */
-  reserved = ArenaReserved(arena);
   res = WriteF(stream, depth + 2,
-               "reserved         $W  <-- "
-               "total size of address-space reserved\n",
-               (WriteFW)reserved,
-               NULL);
-  if (res != ResOK)
-    return res;
-
-  res = WriteF(stream, depth + 2,
-               "committed        $W  <-- "
-               "total bytes currently stored (in RAM or swap)\n",
-               (WriteFW)arena->committed,
+               "reserved         $W\n", (WriteFW)arena->reserved,
+               "committed        $W\n", (WriteFW)arena->committed,
                "commitLimit      $W\n", (WriteFW)arena->commitLimit,
                "spareCommitted   $W\n", (WriteFW)arena->spareCommitted,
                "spareCommitLimit $W\n", (WriteFW)arena->spareCommitLimit,
@@ -669,11 +658,15 @@ Res ControlDescribe(Arena arena, mps_lib_FILE *stream, Count depth)
 }
 
 
-/* ArenaChunkInsert -- insert chunk into arena's chunk tree and ring */
+/* ArenaChunkInsert -- insert chunk into arena's chunk tree and ring,
+ * update the total reserved address space, and set the primary chunk
+ * if not already set.
+ */
 
 void ArenaChunkInsert(Arena arena, Chunk chunk) {
   Bool inserted;
   Tree tree, updatedTree = NULL;
+  Size size;
 
   AVERT(Arena, arena);
   AVERT(Chunk, chunk);
@@ -687,6 +680,9 @@ void ArenaChunkInsert(Arena arena, Chunk chunk) {
   arena->chunkTree = updatedTree;
   RingAppend(&arena->chunkRing, &chunk->arenaRing);
 
+  size = (*arena->class->chunkReserved)(chunk);
+  arena->reserved += size;
+
   /* As part of the bootstrap, the first created chunk becomes the primary
      chunk.  This step allows ArenaFreeLandInsert to allocate pages. */
   if (arena->primary == NULL)
@@ -694,6 +690,27 @@ void ArenaChunkInsert(Arena arena, Chunk chunk) {
 }
 
 
+/* ArenaChunkRemoved -- chunk was removed from the arena and is being
+ * finished, so update the total reserved address space, and unset the
+ * primary chunk if necessary.
+ */
+
+void ArenaChunkRemoved(Arena arena, Chunk chunk)
+{
+  Size size;
+
+  AVERT(Arena, arena);
+  AVERT(Chunk, chunk);
+
+  if (arena->primary == chunk)
+    arena->primary = NULL;
+
+  size = (*arena->class->chunkReserved)(chunk);
+  AVER(arena->reserved >= size);
+  arena->reserved -= size;
+}
+
+
 /* arenaAllocPage -- allocate one page from the arena
  *
  * This is a primitive allocator used to allocate pages for the arena
@@ -1231,7 +1248,7 @@ allDeposited:
 Size ArenaReserved(Arena arena)
 {
   AVERT(Arena, arena);
-  return (*arena->class->reserved)(arena);
+  return arena->reserved;
 }
 
 Size ArenaCommitted(Arena arena)
diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c
index e535ad770cc..685844c3fa4 100644
--- a/mps/code/arenacl.c
+++ b/mps/code/arenacl.c
@@ -206,6 +206,17 @@ static void ClientChunkFinish(Chunk chunk)
 }
 
 
+/* ClientChunkReserved -- return the amount of reserved address space
+ * in a client chunk.
+ */
+
+static Size ClientChunkReserved(Chunk chunk)
+{
+  AVERT(Chunk, chunk);
+  return ChunkSize(chunk);
+}
+
+
 /* ClientArenaVarargs -- parse obsolete varargs */
 
 static void ClientArenaVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
@@ -344,27 +355,6 @@ static Res ClientArenaExtend(Arena arena, Addr base, Size size)
 }
 
 
-/* ClientArenaReserved -- return the amount of reserved address space */
-
-static Size ClientArenaReserved(Arena arena)
-{
-  Size size;
-  Ring node, nextNode;
-
-  AVERT(Arena, arena);
-
-  size = 0;
-  /* .req.extend.slow */
-  RING_FOR(node, &arena->chunkRing, nextNode) {
-    Chunk chunk = RING_ELT(Chunk, arenaRing, node);
-    AVERT(Chunk, chunk);
-    size += ChunkSize(chunk);
-  }
-
-  return size;
-}
-
-
 /* ClientArenaPagesMarkAllocated -- Mark the pages allocated */
 
 static Res ClientArenaPagesMarkAllocated(Arena arena, Chunk chunk,
@@ -447,12 +437,12 @@ DEFINE_ARENA_CLASS(ClientArenaClass, this)
   this->varargs = ClientArenaVarargs;
   this->init = ClientArenaInit;
   this->finish = ClientArenaFinish;
-  this->reserved = ClientArenaReserved;
   this->extend = ClientArenaExtend;
   this->pagesMarkAllocated = ClientArenaPagesMarkAllocated;
   this->free = ClientFree;
   this->chunkInit = ClientChunkInit;
   this->chunkFinish = ClientChunkFinish;
+  this->chunkReserved = ClientChunkReserved;
   AVERT(ArenaClass, this);
 }
 
diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c
index 97d2efe3e95..456876a6312 100644
--- a/mps/code/arenavm.c
+++ b/mps/code/arenavm.c
@@ -456,6 +456,17 @@ static void VMChunkFinish(Chunk chunk)
 }
 
 
+/* VMChunkReserved -- return the amount of reserved address space in a
+ * VM chunk.
+ */
+
+static Size VMChunkReserved(Chunk chunk)
+{
+  AVERT(Chunk, chunk);
+  return VMReserved(VMChunkVM(Chunk2VMChunk(chunk)));
+}
+
+
 /* VMArenaVarargs -- parse obsolete varargs */
 
 static void VMArenaVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
@@ -648,25 +659,6 @@ static void VMArenaFinish(Arena arena)
 }
 
 
-/* VMArenaReserved -- return the amount of reserved address space
- *
- * Add up the reserved space from all the chunks.
- */
-
-static Size VMArenaReserved(Arena arena)
-{
-  Size reserved;
-  Ring node, next;
-
-  reserved = 0;
-  RING_FOR(node, &arena->chunkRing, next) {
-    VMChunk vmChunk = Chunk2VMChunk(RING_ELT(Chunk, arenaRing, node));
-    reserved += VMReserved(VMChunkVM(vmChunk));
-  }
-  return reserved;
-}
-
-
 /* vmArenaChunkSize -- choose chunk size for arena extension
  *
  * .vmchunk.overhead: This code still lacks a proper estimate of
@@ -717,7 +709,7 @@ static Res VMArenaGrow(Arena arena, LocusPref pref, Size size)
   chunkSize = vmArenaChunkSize(vmArena, size);
 
   EVENT3(vmArenaExtendStart, size, chunkSize,
-         VMArenaReserved(VMArena2Arena(vmArena)));
+         ArenaReserved(VMArena2Arena(vmArena)));
 
   /* .chunk-create.fail: If we fail, try again with a smaller size */
   {
@@ -741,7 +733,7 @@ static Res VMArenaGrow(Arena arena, LocusPref pref, Size size)
       for(; chunkSize > chunkHalf; chunkSize -= sliceSize) {
         if(chunkSize < chunkMin) {
           EVENT2(vmArenaExtendFail, chunkMin,
-                 VMArenaReserved(VMArena2Arena(vmArena)));
+                 ArenaReserved(VMArena2Arena(vmArena)));
           return res;
         }
         res = VMChunkCreate(&newChunk, vmArena, chunkSize);
@@ -752,7 +744,7 @@ static Res VMArenaGrow(Arena arena, LocusPref pref, Size size)
   }
   
 vmArenaGrow_Done:
-  EVENT2(vmArenaExtendDone, chunkSize, VMArenaReserved(VMArena2Arena(vmArena)));
+  EVENT2(vmArenaExtendDone, chunkSize, ArenaReserved(VMArena2Arena(vmArena)));
   vmArena->extended(VMArena2Arena(vmArena),
 		    newChunk->base,
 		    AddrOffset(newChunk->base, newChunk->limit));
@@ -1136,7 +1128,7 @@ static void VMCompact(Arena arena, Trace trace)
   AVERT(VMArena, vmArena);
   AVERT(Trace, trace);
 
-  vmem1 = VMArenaReserved(arena);
+  vmem1 = ArenaReserved(arena);
 
   /* Destroy chunks that are completely free, but not the primary
    * chunk. See <design/arena/#chunk.delete>
@@ -1146,7 +1138,7 @@ static void VMCompact(Arena arena, Trace trace)
 
   {
     Size vmem0 = trace->preTraceArenaReserved;
-    Size vmem2 = VMArenaReserved(arena);
+    Size vmem2 = ArenaReserved(arena);
 
     /* VMCompact event: emit for all client-requested collections, */
     /* plus any others where chunks were gained or lost during the */
@@ -1196,12 +1188,12 @@ DEFINE_ARENA_CLASS(VMArenaClass, this)
   this->varargs = VMArenaVarargs;
   this->init = VMArenaInit;
   this->finish = VMArenaFinish;
-  this->reserved = VMArenaReserved;
   this->purgeSpare = VMPurgeSpare;
   this->grow = VMArenaGrow;
   this->free = VMFree;
   this->chunkInit = VMChunkInit;
   this->chunkFinish = VMChunkFinish;
+  this->chunkReserved = VMChunkReserved;
   this->compact = VMCompact;
   this->describe = VMArenaDescribe;
   this->pagesMarkAllocated = VMPagesMarkAllocated;
diff --git a/mps/code/mpm.h b/mps/code/mpm.h
index f1ed6812886..4cf2e26781c 100644
--- a/mps/code/mpm.h
+++ b/mps/code/mpm.h
@@ -566,6 +566,7 @@ extern Res ArenaCollect(Globals globals, int why);
 extern Bool ArenaHasAddr(Arena arena, Addr addr);
 extern Res ArenaAddrObject(Addr *pReturn, Arena arena, Addr addr);
 extern void ArenaChunkInsert(Arena arena, Chunk chunk);
+extern void ArenaChunkRemoved(Arena arena, Chunk chunk);
 
 extern void ArenaSetEmergency(Arena arena, Bool emergency);
 extern Bool ArenaEmergency(Arena arean);
diff --git a/mps/code/mpmst.h b/mps/code/mpmst.h
index ac59492bea5..96e8599e424 100644
--- a/mps/code/mpmst.h
+++ b/mps/code/mpmst.h
@@ -530,13 +530,13 @@ typedef struct mps_arena_class_s {
   ArenaVarargsMethod varargs;
   ArenaInitMethod init;
   ArenaFinishMethod finish;
-  ArenaReservedMethod reserved;
   ArenaPurgeSpareMethod purgeSpare;
   ArenaExtendMethod extend;
   ArenaGrowMethod grow;
   ArenaFreeMethod free;
   ArenaChunkInitMethod chunkInit;
   ArenaChunkFinishMethod chunkFinish;
+  ArenaChunkReservedMethod chunkReserved;
   ArenaCompactMethod compact;
   ArenaDescribeMethod describe;
   ArenaPagesMarkAllocatedMethod pagesMarkAllocated;
@@ -718,7 +718,8 @@ typedef struct mps_arena_s {
 
   ReservoirStruct reservoirStruct; /* <design/reservoir/> */
 
-  Size committed;               /* amount of committed RAM */
+  Size reserved;                /* total reserved address space */
+  Size committed;               /* total committed memory */
   Size commitLimit;             /* client-configurable commit limit */
 
   Size spareCommitted;          /* Amount of memory in hysteresis fund */
diff --git a/mps/code/mpmtypes.h b/mps/code/mpmtypes.h
index 4c0b6bd9c87..de98355342a 100644
--- a/mps/code/mpmtypes.h
+++ b/mps/code/mpmtypes.h
@@ -120,12 +120,12 @@ typedef void (*ArenaVarargsMethod)(ArgStruct args[], va_list varargs);
 typedef Res (*ArenaInitMethod)(Arena *arenaReturn,
                                ArenaClass class, ArgList args);
 typedef void (*ArenaFinishMethod)(Arena arena);
-typedef Size (*ArenaReservedMethod)(Arena arena);
 typedef Size (*ArenaPurgeSpareMethod)(Arena arena, Size size);
 typedef Res (*ArenaExtendMethod)(Arena arena, Addr base, Size size);
 typedef Res (*ArenaGrowMethod)(Arena arena, LocusPref pref, Size size);
 typedef void (*ArenaFreeMethod)(Addr base, Size size, Pool pool);
 typedef Res (*ArenaChunkInitMethod)(Chunk chunk, BootBlock boot);
+typedef Size (*ArenaChunkReservedMethod)(Chunk chunk);
 typedef void (*ArenaChunkFinishMethod)(Chunk chunk);
 typedef void (*ArenaCompactMethod)(Arena arena, Trace trace);
 typedef Res (*ArenaDescribeMethod)(Arena arena, mps_lib_FILE *stream, Count depth);
diff --git a/mps/code/tract.c b/mps/code/tract.c
index 5d3bffe9721..39f6bead95e 100644
--- a/mps/code/tract.c
+++ b/mps/code/tract.c
@@ -262,17 +262,16 @@ void ChunkFinish(Chunk chunk)
                         PageIndexBase(chunk, chunk->allocBase),
                         chunk->limit);
 
+  ArenaChunkRemoved(arena, chunk);
+
   chunk->sig = SigInvalid;
 
   TreeFinish(&chunk->chunkTree);
   RingRemove(&chunk->arenaRing);
 
-  if (chunk->arena->primary == chunk)
-    chunk->arena->primary = NULL;
-
   /* Finish all other fields before class finish, because they might be */
   /* unmapped there. */
-  (chunk->arena->class->chunkFinish)(chunk);
+  (*arena->class->chunkFinish)(chunk);
 }
 
 

From 22aa2a084118b989130c49924866a229b78fd443 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Tue, 30 Sep 2014 11:02:11 +0100
Subject: [PATCH 03/16] Account for the reserved memory in the arena's own vm.
 (this was previously not included in arenareserved.)

Make the computation in pageDescUnmap clearer (and not relying on arithmetic overflow).

Copied from Perforce
 Change: 187091
 ServerID: perforce.ravenbrook.com
---
 mps/code/arenavm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c
index 456876a6312..fe1f0f91e9a 100644
--- a/mps/code/arenavm.c
+++ b/mps/code/arenavm.c
@@ -565,6 +565,7 @@ static Res VMArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args)
   res = ArenaInit(arena, class, grainSize, args);
   if (res != ResOK)
     goto failArenaInit;
+  arena->reserved = VMReserved(vm);
   arena->committed = VMMapped(vm);
 
   /* Copy VM descriptor into its place in the arena. */
@@ -645,6 +646,7 @@ static void VMArenaFinish(Arena arena)
   RingFinish(&vmArena->spareRing);
 
   /* Destroying the chunks should leave only the arena's own VM. */
+  AVER(arena->reserved == VMReserved(VMArenaVM(vmArena)));
   AVER(arena->committed == VMMapped(VMArenaVM(vmArena)));
 
   vmArena->sig = SigInvalid;
@@ -798,10 +800,13 @@ static Res pageDescMap(VMChunk vmChunk, Index basePI, Index limitPI)
 
 static void pageDescUnmap(VMChunk vmChunk, Index basePI, Index limitPI)
 {
+  Size size;
   Size before = VMMapped(VMChunkVM(vmChunk));
   Arena arena = VMArena2Arena(VMChunkVMArena(vmChunk));
   SparseArrayUnmap(&vmChunk->pages, basePI, limitPI);
-  arena->committed += VMMapped(VMChunkVM(vmChunk)) - before;
+  size = before - VMMapped(VMChunkVM(vmChunk));
+  AVER(arena->committed >= size);
+  arena->committed -= size;
 }
 
 

From 05408fa99236e0d0c8ac669ede374613fc4bbefa Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Tue, 30 Sep 2014 18:57:50 +0100
Subject: [PATCH 04/16] Remove orphaned comment (originally from grm in change
 19855) about being unable to "check that limit>=size" (referring to the
 reservoir). it seems, at least from testing, that this check is good now, so
 add it to reservoircheck.

Copied from Perforce
 Change: 187092
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c  | 3 ---
 mps/code/reserv.c | 1 +
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index e8efe5d9bdb..c42b7e56cf9 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -142,9 +142,6 @@ Bool ArenaCheck(Arena arena)
     CHECKD(Reservoir, &arena->reservoirStruct);
   }
 
-  /* Can't check that limit>=size because we may call ArenaCheck */
-  /* while the size is being adjusted. */
-
   CHECKL(arena->committed <= arena->commitLimit);
   CHECKL(arena->spareCommitted <= arena->committed);
 
diff --git a/mps/code/reserv.c b/mps/code/reserv.c
index b0d431c3f9c..91a2fd52559 100644
--- a/mps/code/reserv.c
+++ b/mps/code/reserv.c
@@ -100,6 +100,7 @@ Bool ReservoirCheck(Reservoir reservoir)
   }
   CHECKL(SizeIsArenaGrains(reservoir->reservoirLimit, arena));
   CHECKL(SizeIsArenaGrains(reservoir->reservoirSize, arena));
+  CHECKL(reservoir->reservoirSize <= reservoir->reservoirLimit);
 
   return TRUE;
 }

From f01b9b7bb58267560cd6a28011c2318b1cafafef Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Tue, 30 Sep 2014 20:04:35 +0100
Subject: [PATCH 05/16] Document the behaviour of committed (memory marked as
 in use) and spare_committed (always zero) in the client arena class.

Account for (and check) committed memory in the client arena class.
Can't check arena->committed <= arena->reserved in ArenaCheck, but can in ClientArenaCheck.
Rename ClientFree to ClientArenaFree to match the naming convention elsewhere.

Copied from Perforce
 Change: 187093
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c                  |  6 +++++
 mps/code/arenacl.c                | 41 +++++++++++++++++++++++++------
 mps/manual/source/topic/arena.rst | 30 ++++++++++++++++------
 3 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index c42b7e56cf9..3a41ebbb0df 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -142,6 +142,12 @@ Bool ArenaCheck(Arena arena)
     CHECKD(Reservoir, &arena->reservoirStruct);
   }
 
+  /* .reserved.check: Would like to check that arena->committed <=
+   * arena->reserved, but that isn't always true in the VM arena.
+   * Memory is committed early on when VMChunkCreate calls VMArenaMap
+   * (to provide a place for the chunk struct) but is not recorded as
+   * reserved until ChunkInit calls ArenaChunkInsert.
+   */
   CHECKL(arena->committed <= arena->commitLimit);
   CHECKL(arena->spareCommitted <= arena->committed);
 
diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c
index 685844c3fa4..554dc963c72 100644
--- a/mps/code/arenacl.c
+++ b/mps/code/arenacl.c
@@ -81,8 +81,15 @@ static Bool ClientChunkCheck(ClientChunk clChunk)
 ATTRIBUTE_UNUSED
 static Bool ClientArenaCheck(ClientArena clientArena)
 {
+  Arena arena;
+
   CHECKS(ClientArena, clientArena);
-  CHECKD(Arena, ClientArena2Arena(clientArena));
+  arena = ClientArena2Arena(clientArena);
+  CHECKD(Arena, arena);
+  /* See <code/arena.c#.reserved.check> */
+  CHECKL(arena->committed <= arena->reserved);
+  CHECKL(arena->spareCommitted == 0);
+
   return TRUE;
 }
 
@@ -128,8 +135,8 @@ static Res clientChunkCreate(Chunk *chunkReturn, ClientArena clientArena,
   if (res != ResOK)
     goto failChunkInit;
 
-  ClientArena2Arena(clientArena)->committed +=
-    AddrOffset(base, PageIndexBase(chunk, chunk->allocBase));
+  arena->committed += ChunkPagesToSize(chunk, chunk->allocBase);
+
   BootBlockFinish(boot);
 
   clChunk->sig = ClientChunkSig;
@@ -176,8 +183,10 @@ static Res ClientChunkInit(Chunk chunk, BootBlock boot)
 
 static Bool clientChunkDestroy(Tree tree, void *closureP, Size closureS)
 {
+  Arena arena;
   Chunk chunk;
   ClientChunk clChunk;
+  Size size;
 
   AVERT(Tree, tree);
   AVER(closureP == UNUSED_POINTER);
@@ -187,8 +196,15 @@ static Bool clientChunkDestroy(Tree tree, void *closureP, Size closureS)
   
   chunk = ChunkOfTree(tree);
   AVERT(Chunk, chunk);
+  arena = ChunkArena(chunk);  
+  AVERT(Arena, arena);
   clChunk = Chunk2ClientChunk(chunk);
   AVERT(ClientChunk, clChunk);
+  AVER(chunk->pages == clChunk->freePages);
+
+  size = ChunkPagesToSize(chunk, chunk->allocBase);
+  AVER(arena->committed >= size);
+  arena->committed -= size;
 
   clChunk->sig = SigInvalid;
   ChunkFinish(chunk);
@@ -331,6 +347,10 @@ static void ClientArenaFinish(Arena arena)
 
   clientArena->sig = SigInvalid;
 
+  /* Destroying the chunks should leave nothing behind. */
+  AVER(arena->reserved == 0);
+  AVER(arena->committed == 0);
+
   ArenaFinish(arena); /* <code/arena.c#finish.caller> */
 }
 
@@ -362,9 +382,12 @@ static Res ClientArenaPagesMarkAllocated(Arena arena, Chunk chunk,
                                          Pool pool)
 {
   Index i;
+  ClientChunk clChunk;
   
   AVERT(Arena, arena);
   AVERT(Chunk, chunk);
+  clChunk = Chunk2ClientChunk(chunk);
+  AVERT(ClientChunk, clChunk);
   AVER(chunk->allocBase <= baseIndex);
   AVER(pages > 0);
   AVER(baseIndex + pages <= chunk->pages);
@@ -373,15 +396,17 @@ static Res ClientArenaPagesMarkAllocated(Arena arena, Chunk chunk,
   for (i = 0; i < pages; ++i)
     PageAlloc(chunk, baseIndex + i, pool);
 
-  Chunk2ClientChunk(chunk)->freePages -= pages;
+  arena->committed += ChunkPagesToSize(chunk, pages);
+  AVER(clChunk->freePages >= pages);
+  clChunk->freePages -= pages;
 
   return ResOK;
 }
 
 
-/* ClientFree - free a region in the arena */
+/* ClientArenaFree - free a region in the arena */
 
-static void ClientFree(Addr base, Size size, Pool pool)
+static void ClientArenaFree(Addr base, Size size, Pool pool)
 {
   Arena arena;
   Chunk chunk = NULL;           /* suppress "may be used uninitialized" */
@@ -422,6 +447,8 @@ static void ClientFree(Addr base, Size size, Pool pool)
   AVER(BTIsSetRange(chunk->allocTable, baseIndex, limitIndex));
   BTResRange(chunk->allocTable, baseIndex, limitIndex);
 
+  AVER(arena->committed >= size);
+  arena->committed -= size;
   clChunk->freePages += pages;
 }
 
@@ -439,7 +466,7 @@ DEFINE_ARENA_CLASS(ClientArenaClass, this)
   this->finish = ClientArenaFinish;
   this->extend = ClientArenaExtend;
   this->pagesMarkAllocated = ClientArenaPagesMarkAllocated;
-  this->free = ClientFree;
+  this->free = ClientArenaFree;
   this->chunkInit = ClientChunkInit;
   this->chunkFinish = ClientChunkFinish;
   this->chunkReserved = ClientChunkReserved;
diff --git a/mps/manual/source/topic/arena.rst b/mps/manual/source/topic/arena.rst
index ce313166161..a534135b9c8 100644
--- a/mps/manual/source/topic/arena.rst
+++ b/mps/manual/source/topic/arena.rst
@@ -380,9 +380,18 @@ Arena properties
 
     ``arena`` is the arena.
 
-    Returns the total amount of memory that has been committed to RAM
+    Returns the total amount of memory that has been committed for use
     by the MPS, in :term:`bytes (1)`.
 
+    For a :term:`virtual memory arena`, this is the amount of memory
+    mapped to RAM by the operating system's virtual memory interface.
+
+    For a :term:`client arena`, this is the amount of memory marked as
+    in use in the arena's page tables. This is not particularly
+    meaningful by itself, but it corresponds to the amount of mapped
+    memory that the MPS would use if switched to a virtual memory
+    arena.
+
     The committed memory is generally larger than the sum of the sizes
     of the allocated :term:`blocks`. The reasons for this are:
 
@@ -412,7 +421,7 @@ Arena properties
     (over-)estimate the size of the heap.
 
     If you want to know how much memory the MPS is using then you're
-    probably interested in the value :c:func:`mps_arena_committed()` −
+    probably interested in the value :c:func:`mps_arena_committed` −
     :c:func:`mps_arena_spare_committed`.
 
     The amount of committed memory can be limited with the function
@@ -436,12 +445,12 @@ Arena properties
 
     .. note::
 
-        For a client arena, the reserved address may be lower than the
-        sum of the :c:macro:`MPS_KEY_ARENA_SIZE` keyword argument
-        passed to :c:func:`mps_arena_create_k` and the ``size``
-        arguments passed to :c:func:`mps_arena_extend`, because the
-        arena may be unable to use the whole of each chunk for reasons
-        of alignment.
+        For a :term:`client arena`, the reserved address may be lower
+        than the sum of the :c:macro:`MPS_KEY_ARENA_SIZE` keyword
+        argument passed to :c:func:`mps_arena_create_k` and the
+        ``size`` arguments passed to :c:func:`mps_arena_extend`,
+        because the arena may be unable to use the whole of each chunk
+        for reasons of alignment.
 
 
 .. c:function:: size_t mps_arena_spare_commit_limit(mps_arena_t arena)
@@ -506,6 +515,11 @@ Arena properties
     functions for limiting the amount of :term:`committed <mapped>`
     memory.
 
+    .. note::
+
+        :term:`Client arenas` do not use spare committed memory, and
+        so this function always returns 0.
+
 
 .. index::
    single: arena; states

From 9ca8dc11bf8436f392a2fe8cc2ab2233e35f9080 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Tue, 30 Sep 2014 20:21:21 +0100
Subject: [PATCH 06/16] Update release notes for job001887.

Copied from Perforce
 Change: 187096
 ServerID: perforce.ravenbrook.com
---
 mps/manual/source/release.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mps/manual/source/release.rst b/mps/manual/source/release.rst
index fc9cc438c55..070980df085 100644
--- a/mps/manual/source/release.rst
+++ b/mps/manual/source/release.rst
@@ -17,6 +17,16 @@ Interface changes
    but is deprecated.
 
 
+Other changes
+.............
+
+#. :c:func:`mps_arena_committed` now returns a meaningful value (the
+   amount of memory marked as in use in the page tables) for
+   :term:`client arenas`. See job001887_.
+
+   .. _job001887: https://www.ravenbrook.com/project/mps/issue/job001887/
+
+
 .. _release-notes-1.114:
 
 Release 1.114.0

From 943471d0157e562103135b10d4a916fe924fd07e Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Wed, 1 Oct 2014 21:40:50 +0100
Subject: [PATCH 07/16] Store reserved address space associated with chunk in a
 field in the chunkstruct, as suggested by rb in
 <https://info.ravenbrook.com/mail/2014/10/01/13-55-44/0/>.

Copied from Perforce
 Change: 187104
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c    |  8 ++------
 mps/code/arenacl.c  | 15 ++-------------
 mps/code/arenavm.c  | 15 ++-------------
 mps/code/mpmst.h    |  1 -
 mps/code/mpmtypes.h |  1 -
 mps/code/tract.c    |  4 +++-
 mps/code/tract.h    |  6 +++++-
 7 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index 3a41ebbb0df..05279ab6bd6 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -91,7 +91,6 @@ DEFINE_CLASS(AbstractArenaClass, class)
   class->free = NULL;
   class->chunkInit = NULL;
   class->chunkFinish = NULL;
-  class->chunkReserved = NULL;
   class->compact = ArenaTrivCompact;
   class->describe = ArenaTrivDescribe;
   class->pagesMarkAllocated = NULL;
@@ -119,7 +118,6 @@ Bool ArenaClassCheck(ArenaClass class)
   CHECKL(FUNCHECK(class->free));
   CHECKL(FUNCHECK(class->chunkInit));
   CHECKL(FUNCHECK(class->chunkFinish));
-  CHECKL(FUNCHECK(class->chunkReserved));
   CHECKL(FUNCHECK(class->compact));
   CHECKL(FUNCHECK(class->describe));
   CHECKL(FUNCHECK(class->pagesMarkAllocated));
@@ -669,7 +667,6 @@ Res ControlDescribe(Arena arena, mps_lib_FILE *stream, Count depth)
 void ArenaChunkInsert(Arena arena, Chunk chunk) {
   Bool inserted;
   Tree tree, updatedTree = NULL;
-  Size size;
 
   AVERT(Arena, arena);
   AVERT(Chunk, chunk);
@@ -683,8 +680,7 @@ void ArenaChunkInsert(Arena arena, Chunk chunk) {
   arena->chunkTree = updatedTree;
   RingAppend(&arena->chunkRing, &chunk->arenaRing);
 
-  size = (*arena->class->chunkReserved)(chunk);
-  arena->reserved += size;
+  arena->reserved += ChunkReserved(chunk);
 
   /* As part of the bootstrap, the first created chunk becomes the primary
      chunk.  This step allows ArenaFreeLandInsert to allocate pages. */
@@ -708,7 +704,7 @@ void ArenaChunkRemoved(Arena arena, Chunk chunk)
   if (arena->primary == chunk)
     arena->primary = NULL;
 
-  size = (*arena->class->chunkReserved)(chunk);
+  size = ChunkReserved(chunk);
   AVER(arena->reserved >= size);
   arena->reserved -= size;
 }
diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c
index 554dc963c72..5212faf702b 100644
--- a/mps/code/arenacl.c
+++ b/mps/code/arenacl.c
@@ -131,7 +131,8 @@ static Res clientChunkCreate(Chunk *chunkReturn, ClientArena clientArena,
   clChunk = p;  chunk = ClientChunk2Chunk(clChunk);
 
   res = ChunkInit(chunk, arena, alignedBase,
-                  AddrAlignDown(limit, ArenaGrainSize(arena)), boot);
+                  AddrAlignDown(limit, ArenaGrainSize(arena)),
+                  AddrOffset(base, limit), boot);
   if (res != ResOK)
     goto failChunkInit;
 
@@ -222,17 +223,6 @@ static void ClientChunkFinish(Chunk chunk)
 }
 
 
-/* ClientChunkReserved -- return the amount of reserved address space
- * in a client chunk.
- */
-
-static Size ClientChunkReserved(Chunk chunk)
-{
-  AVERT(Chunk, chunk);
-  return ChunkSize(chunk);
-}
-
-
 /* ClientArenaVarargs -- parse obsolete varargs */
 
 static void ClientArenaVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
@@ -469,7 +459,6 @@ DEFINE_ARENA_CLASS(ClientArenaClass, this)
   this->free = ClientArenaFree;
   this->chunkInit = ClientChunkInit;
   this->chunkFinish = ClientChunkFinish;
-  this->chunkReserved = ClientChunkReserved;
   AVERT(ArenaClass, this);
 }
 
diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c
index fe1f0f91e9a..b37e7125fdd 100644
--- a/mps/code/arenavm.c
+++ b/mps/code/arenavm.c
@@ -323,7 +323,8 @@ static Res VMChunkCreate(Chunk *chunkReturn, VMArena vmArena, Size size)
 
   /* Copy VM descriptor into its place in the chunk. */
   VMCopy(VMChunkVM(vmChunk), vm);
-  res = ChunkInit(VMChunk2Chunk(vmChunk), arena, base, limit, boot);
+  res = ChunkInit(VMChunk2Chunk(vmChunk), arena, base, limit,
+                  VMReserved(VMChunkVM(vmChunk)), boot);
   if (res != ResOK)
     goto failChunkInit;
 
@@ -456,17 +457,6 @@ static void VMChunkFinish(Chunk chunk)
 }
 
 
-/* VMChunkReserved -- return the amount of reserved address space in a
- * VM chunk.
- */
-
-static Size VMChunkReserved(Chunk chunk)
-{
-  AVERT(Chunk, chunk);
-  return VMReserved(VMChunkVM(Chunk2VMChunk(chunk)));
-}
-
-
 /* VMArenaVarargs -- parse obsolete varargs */
 
 static void VMArenaVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
@@ -1198,7 +1188,6 @@ DEFINE_ARENA_CLASS(VMArenaClass, this)
   this->free = VMFree;
   this->chunkInit = VMChunkInit;
   this->chunkFinish = VMChunkFinish;
-  this->chunkReserved = VMChunkReserved;
   this->compact = VMCompact;
   this->describe = VMArenaDescribe;
   this->pagesMarkAllocated = VMPagesMarkAllocated;
diff --git a/mps/code/mpmst.h b/mps/code/mpmst.h
index 96e8599e424..fecd90afa54 100644
--- a/mps/code/mpmst.h
+++ b/mps/code/mpmst.h
@@ -536,7 +536,6 @@ typedef struct mps_arena_class_s {
   ArenaFreeMethod free;
   ArenaChunkInitMethod chunkInit;
   ArenaChunkFinishMethod chunkFinish;
-  ArenaChunkReservedMethod chunkReserved;
   ArenaCompactMethod compact;
   ArenaDescribeMethod describe;
   ArenaPagesMarkAllocatedMethod pagesMarkAllocated;
diff --git a/mps/code/mpmtypes.h b/mps/code/mpmtypes.h
index de98355342a..b0d1fe2003a 100644
--- a/mps/code/mpmtypes.h
+++ b/mps/code/mpmtypes.h
@@ -125,7 +125,6 @@ typedef Res (*ArenaExtendMethod)(Arena arena, Addr base, Size size);
 typedef Res (*ArenaGrowMethod)(Arena arena, LocusPref pref, Size size);
 typedef void (*ArenaFreeMethod)(Addr base, Size size, Pool pool);
 typedef Res (*ArenaChunkInitMethod)(Chunk chunk, BootBlock boot);
-typedef Size (*ArenaChunkReservedMethod)(Chunk chunk);
 typedef void (*ArenaChunkFinishMethod)(Chunk chunk);
 typedef void (*ArenaCompactMethod)(Arena arena, Trace trace);
 typedef Res (*ArenaDescribeMethod)(Arena arena, mps_lib_FILE *stream, Count depth);
diff --git a/mps/code/tract.c b/mps/code/tract.c
index 39f6bead95e..9aa815f47ba 100644
--- a/mps/code/tract.c
+++ b/mps/code/tract.c
@@ -167,7 +167,8 @@ Bool ChunkCheck(Chunk chunk)
 
 /* ChunkInit -- initialize generic part of chunk */
 
-Res ChunkInit(Chunk chunk, Arena arena, Addr base, Addr limit, BootBlock boot)
+Res ChunkInit(Chunk chunk, Arena arena, Addr base, Addr limit, Size reserved,
+              BootBlock boot)
 {
   Size size;
   Count pages;
@@ -192,6 +193,7 @@ Res ChunkInit(Chunk chunk, Arena arena, Addr base, Addr limit, BootBlock boot)
   chunk->pageShift = pageShift = SizeLog2(chunk->pageSize);
   chunk->base = base;
   chunk->limit = limit;
+  chunk->reserved = reserved;
   size = ChunkSize(chunk);
 
   chunk->pages = pages = size >> pageShift;
diff --git a/mps/code/tract.h b/mps/code/tract.h
index 07906ad5756..ce7c602a176 100644
--- a/mps/code/tract.h
+++ b/mps/code/tract.h
@@ -148,6 +148,9 @@ typedef struct ChunkStruct {
   BT allocTable;        /* page allocation table */
   Page pageTable;       /* the page table */
   Count pageTablePages; /* number of pages occupied by page table */
+  Size reserved;        /* reserved address space for chunk (including overhead
+                           such as losses due to alignment): must not change
+                           (or arena reserved calculation will break) */
 } ChunkStruct;
 
 
@@ -159,10 +162,11 @@ typedef struct ChunkStruct {
 #define ChunkSizeToPages(chunk, size) ((Count)((size) >> (chunk)->pageShift))
 #define ChunkPage(chunk, pi) (&(chunk)->pageTable[pi])
 #define ChunkOfTree(tree) PARENT(ChunkStruct, chunkTree, tree)
+#define ChunkReserved(chunk) RVALUE((chunk)->reserved)
 
 extern Bool ChunkCheck(Chunk chunk);
 extern Res ChunkInit(Chunk chunk, Arena arena, Addr base, Addr limit,
-                     BootBlock boot);
+                     Size reserved, BootBlock boot);
 extern void ChunkFinish(Chunk chunk);
 extern Compare ChunkCompare(Tree tree, TreeKey key);
 extern TreeKey ChunkKey(Tree tree);

From e2e51376f9b71d5f76b56dc96726a711603d4530 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Thu, 6 Nov 2014 10:51:18 +0000
Subject: [PATCH 08/16] Assert that sparsearrayunmap made some (or no)
 progress.

Copied from Perforce
 Change: 187464
 ServerID: perforce.ravenbrook.com
---
 mps/code/arenavm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c
index b37e7125fdd..7c4f44aa127 100644
--- a/mps/code/arenavm.c
+++ b/mps/code/arenavm.c
@@ -790,11 +790,13 @@ static Res pageDescMap(VMChunk vmChunk, Index basePI, Index limitPI)
 
 static void pageDescUnmap(VMChunk vmChunk, Index basePI, Index limitPI)
 {
-  Size size;
+  Size size, after;
   Size before = VMMapped(VMChunkVM(vmChunk));
   Arena arena = VMArena2Arena(VMChunkVMArena(vmChunk));
   SparseArrayUnmap(&vmChunk->pages, basePI, limitPI);
-  size = before - VMMapped(VMChunkVM(vmChunk));
+  after = VMMapped(VMChunkVM(vmChunk));
+  AVER(after <= before);
+  size = before - after;
   AVER(arena->committed >= size);
   arena->committed -= size;
 }

From 2dc79898eb48b97a238c1e148cb3099385939910 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Thu, 6 Nov 2014 10:56:45 +0000
Subject: [PATCH 09/16] Check that sparsearraymap makes some (or no) progress.

Copied from Perforce
 Change: 187465
 ServerID: perforce.ravenbrook.com
---
 mps/code/arenavm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c
index 7c4f44aa127..81bea846cb7 100644
--- a/mps/code/arenavm.c
+++ b/mps/code/arenavm.c
@@ -784,7 +784,9 @@ static Res pageDescMap(VMChunk vmChunk, Index basePI, Index limitPI)
   Size before = VMMapped(VMChunkVM(vmChunk));
   Arena arena = VMArena2Arena(VMChunkVMArena(vmChunk));
   Res res = SparseArrayMap(&vmChunk->pages, basePI, limitPI);
-  arena->committed += VMMapped(VMChunkVM(vmChunk)) - before;
+  Size after = VMMapped(VMChunkVM(vmChunk));
+  AVER(before <= after);
+  arena->committed += after - before;
   return res;
 }
 

From 22769d3828a71bf07aa92afc813e3b8809eab3a6 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Thu, 5 Feb 2015 10:49:20 +0000
Subject: [PATCH 10/16] Add omitted word.

Copied from Perforce
 Change: 187673
 ServerID: perforce.ravenbrook.com
---
 mps/manual/source/topic/arena.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mps/manual/source/topic/arena.rst b/mps/manual/source/topic/arena.rst
index a534135b9c8..53f5d5d3fc0 100644
--- a/mps/manual/source/topic/arena.rst
+++ b/mps/manual/source/topic/arena.rst
@@ -445,10 +445,10 @@ Arena properties
 
     .. note::
 
-        For a :term:`client arena`, the reserved address may be lower
-        than the sum of the :c:macro:`MPS_KEY_ARENA_SIZE` keyword
-        argument passed to :c:func:`mps_arena_create_k` and the
-        ``size`` arguments passed to :c:func:`mps_arena_extend`,
+        For a :term:`client arena`, the reserved address space may be
+        lower than the sum of the :c:macro:`MPS_KEY_ARENA_SIZE`
+        keyword argument passed to :c:func:`mps_arena_create_k` and
+        the ``size`` arguments passed to :c:func:`mps_arena_extend`,
         because the arena may be unable to use the whole of each chunk
         for reasons of alignment.
 

From 413a36fbcff00296d798ba3a450648441e1199aa Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Fri, 19 Jun 2015 12:01:01 +0100
Subject: [PATCH 11/16] Fix comment (function is named vmarenamap, not
 vmarenamap).

Copied from Perforce
 Change: 187965
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index 05279ab6bd6..6327b791b59 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -142,7 +142,7 @@ Bool ArenaCheck(Arena arena)
 
   /* .reserved.check: Would like to check that arena->committed <=
    * arena->reserved, but that isn't always true in the VM arena.
-   * Memory is committed early on when VMChunkCreate calls VMArenaMap
+   * Memory is committed early on when VMChunkCreate calls vmArenaMap
    * (to provide a place for the chunk struct) but is not recorded as
    * reserved until ChunkInit calls ArenaChunkInsert.
    */

From 9863533bfc2a2b0b94b7b661ddd70e4e513c7af4 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Fri, 7 Aug 2015 16:14:50 +0100
Subject: [PATCH 12/16] The primary chunk is always the last chunk to be
 removed, so assert that. review suggestion from rb; see
 <https://info.ravenbrook.com/mail/2015/08/07/13-56-51/0/>

Copied from Perforce
 Change: 188089
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index 6327b791b59..95355935c57 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -701,12 +701,16 @@ void ArenaChunkRemoved(Arena arena, Chunk chunk)
   AVERT(Arena, arena);
   AVERT(Chunk, chunk);
 
-  if (arena->primary == chunk)
-    arena->primary = NULL;
-
   size = ChunkReserved(chunk);
   AVER(arena->reserved >= size);
   arena->reserved -= size;
+
+  if (chunk == arena->primary) {
+    /* The primary chunk must be the last chunk to be removed. */
+    AVER(RingIsSingle(&arena->chunkRing));
+    AVER(arena->reserved == 0);
+    arena->primary = NULL;
+  }
 }
 
 

From 742e171c6bb24c8b937b23fc6d631c746fdce1dc Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 10 Aug 2015 10:57:39 +0100
Subject: [PATCH 13/16] Branching master to branch/2015-08-10/arena-create.

Copied from Perforce
 Change: 188096
 ServerID: perforce.ravenbrook.com

From c966e6c33e546593b654d61465a01968ff547702 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 10 Aug 2015 11:41:58 +0100
Subject: [PATCH 14/16] Correct the test for too-small client arena sizes.

Add automated test case for client arenas with small sizes.

Copied from Perforce
 Change: 188099
 ServerID: perforce.ravenbrook.com
---
 mps/code/arenacl.c      |  2 +-
 mps/test/function/121.c | 50 +++++++++++++++++++++++------------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c
index 2a639c99baf..cd5c24fe46a 100644
--- a/mps/code/arenacl.c
+++ b/mps/code/arenacl.c
@@ -274,7 +274,7 @@ static Res ClientArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args)
   AVER(base != (Addr)0);
   AVERT(ArenaGrainSize, grainSize);
 
-  if (size < grainSize * MPS_WORD_SHIFT)
+  if (size < grainSize * MPS_WORD_WIDTH)
     /* Not enough room for a full complement of zones. */
     return ResMEMORY;
 
diff --git a/mps/test/function/121.c b/mps/test/function/121.c
index 263fa705594..c5780e23ee7 100644
--- a/mps/test/function/121.c
+++ b/mps/test/function/121.c
@@ -11,50 +11,56 @@ END_HEADER
 
 #include "testlib.h"
 #include "mpsavm.h"
-#include "mpscmv.h"
-
-
-void *stackpointer;
+#include "mpsacl.h"
 
 mps_arena_t arena;
-mps_thr_t thread;
-mps_pool_t pool;
-mps_pool_t pools[100];
 
+static char buffer[1024 * 1024];
 
 static void test(void)
 {
+  mps_res_t res, prev_res = MPS_RES_OK;
   int i;
-  for (i = 64; i >= 0; i--) {
-    mps_res_t res;
+  
+  /* VM arenas round up small sizes and so creation must succeed. */
+  for (i = 1024; i >= 0; i -= i/17 + 1) {
+    MPS_ARGS_BEGIN(args) {
+      MPS_ARGS_ADD(args, MPS_KEY_ARENA_SIZE, 1024 * i);
+      die(mps_arena_create_k(&arena, mps_arena_class_vm(), args),
+          "mps_arena_create");
+    } MPS_ARGS_END(args);
+    mps_arena_destroy(arena);
+  }
 
-    comment("Trying arena of %d kB.", i);
-    res = mps_arena_create(&arena, mps_arena_class_vm(), (size_t)(1024*i));
+  /* Client arenas have to work within the memory they are given and
+   * so must fail at some point. */
+  for (i = 1024; i >= 0; i -= i/17 + 1) {
+    MPS_ARGS_BEGIN(args) {
+      MPS_ARGS_ADD(args, MPS_KEY_ARENA_CL_BASE, buffer);
+      MPS_ARGS_ADD(args, MPS_KEY_ARENA_SIZE, 1024 * i);
+      res = mps_arena_create_k(&arena, mps_arena_class_cl(), args);
+    } MPS_ARGS_END(args);
     if (res == MPS_RES_OK) {
-      res = mps_thread_reg(&thread, arena);
-      if (res == MPS_RES_OK) {
-        mps_thread_dereg(thread);
-      } else {
-        if (res != MPS_RES_MEMORY) {
-          error("Wrong error code, %d, for mps_thread_reg.", res);
-        }
+      if (prev_res != MPS_RES_OK) {
+        error("Success with smaller size.");
       }
       mps_arena_destroy(arena);
     } else {
-      report_res("arena_create", res);
       if (res != MPS_RES_MEMORY) {
+        report_res("arena_create", res);
         error("Wrong error code.");
       }
     }
+    prev_res = res;
+  }
+  if (res != MPS_RES_MEMORY) {
+    error("Wrong error code.");
   }
 }
 
 
 int main(void)
 {
- void *m;
- stackpointer=&m; /* hack to get stack pointer */
-
  easy_tramp(test);
  pass();
  return 0;

From a834298be64ca6c87cfa7f2611ed26aa967bc56b Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 10 Aug 2015 11:43:17 +0100
Subject: [PATCH 15/16] Tear down arena correctly if controlinit fails.

Copied from Perforce
 Change: 188100
 ServerID: perforce.ravenbrook.com
---
 mps/code/arena.c | 62 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/mps/code/arena.c b/mps/code/arena.c
index fee56d6378a..31d98251f96 100644
--- a/mps/code/arena.c
+++ b/mps/code/arena.c
@@ -41,6 +41,7 @@ Bool ArenaGrainSizeCheck(Size size)
 
 static void ArenaTrivCompact(Arena arena, Trace trace);
 static void arenaFreePage(Arena arena, Addr base, Pool pool);
+static void arenaFreeLandFinish(Arena arena);
 
 
 /* ArenaTrivDescribe -- produce trivial description of an arena */
@@ -301,6 +302,26 @@ ARG_DEFINE_KEY(ARENA_SIZE, Size);
 ARG_DEFINE_KEY(ARENA_GRAIN_SIZE, Size);
 ARG_DEFINE_KEY(ARENA_ZONED, Bool);
 
+static Res arenaFreeLandInit(Arena arena)
+{
+  Res res;
+
+  AVERT(Arena, arena);
+  AVER(!arena->hasFreeLand);
+  AVER(arena->primary != NULL);
+
+  /* With the primary chunk initialised we can add page memory to the freeLand
+   * that describes the free address space in the primary chunk. */
+  res = ArenaFreeLandInsert(arena,
+                            PageIndexBase(arena->primary,
+                                          arena->primary->allocBase),
+                            arena->primary->limit);
+  if (res != ResOK)
+    return res;
+  arena->hasFreeLand = TRUE;
+  return ResOK;
+}
+
 Res ArenaCreate(Arena *arenaReturn, ArenaClass class, ArgList args)
 {
   Arena arena;
@@ -326,15 +347,9 @@ Res ArenaCreate(Arena *arenaReturn, ArenaClass class, ArgList args)
     goto failStripeSize;
   }
 
-  /* With the primary chunk initialised we can add page memory to the freeLand
-     that describes the free address space in the primary chunk. */
-  res = ArenaFreeLandInsert(arena,
-                            PageIndexBase(arena->primary,
-                                          arena->primary->allocBase),
-                            arena->primary->limit);
+  res = arenaFreeLandInit(arena);
   if (res != ResOK)
-    goto failPrimaryLand;
-  arena->hasFreeLand = TRUE;
+    goto failFreeLandInit;
   
   res = ControlInit(arena);
   if (res != ResOK)
@@ -351,7 +366,8 @@ Res ArenaCreate(Arena *arenaReturn, ArenaClass class, ArgList args)
 failGlobalsCompleteCreate:
   ControlFinish(arena);
 failControlInit:
-failPrimaryLand:
+  arenaFreeLandFinish(arena);
+failFreeLandInit:
 failStripeSize:
   (*class->finish)(arena);
 failInit:
@@ -392,6 +408,20 @@ static void arenaMFSPageFreeVisitor(Pool pool, Addr base, Size size,
   arenaFreePage(PoolArena(pool), base, pool);
 }
 
+static void arenaFreeLandFinish(Arena arena)
+{
+  /* We must tear down the freeLand before the chunks, because pages
+   * containing CBS blocks might be allocated in those chunks. */
+  AVER(arena->hasFreeLand);
+  arena->hasFreeLand = FALSE;
+  LandFinish(ArenaFreeLand(arena));
+
+  /* The CBS block pool can't free its own memory via ArenaFree because
+   * that would use the freeLand. */
+  MFSFinishTracts(ArenaCBSBlockPool(arena), arenaMFSPageFreeVisitor,
+                  UNUSED_POINTER, UNUSED_SIZE);
+}
+
 void ArenaDestroy(Arena arena)
 {
   AVERT(Arena, arena);
@@ -401,19 +431,9 @@ void ArenaDestroy(Arena arena)
   /* Empty the reservoir - see <code/reserv.c#reservoir.finish> */
   ReservoirSetLimit(ArenaReservoir(arena), 0);
 
-  arena->poolReady = FALSE;
   ControlFinish(arena);
 
-  /* We must tear down the freeLand before the chunks, because pages
-     containing CBS blocks might be allocated in those chunks. */
-  AVER(arena->hasFreeLand);
-  arena->hasFreeLand = FALSE;
-  LandFinish(ArenaFreeLand(arena));
-
-  /* The CBS block pool can't free its own memory via ArenaFree because
-     that would use the freeLand. */
-  MFSFinishTracts(ArenaCBSBlockPool(arena), arenaMFSPageFreeVisitor,
-                  UNUSED_POINTER, UNUSED_SIZE);
+  arenaFreeLandFinish(arena);
 
   /* Call class-specific finishing.  This will call ArenaFinish. */
   (*arena->class->finish)(arena);
@@ -429,6 +449,7 @@ Res ControlInit(Arena arena)
   Res res;
 
   AVERT(Arena, arena);
+  AVER(!arena->poolReady);
   MPS_ARGS_BEGIN(args) {
     MPS_ARGS_ADD(args, MPS_KEY_EXTEND_BY, CONTROL_EXTEND_BY);
     res = PoolInit(MVPool(&arena->controlPoolStruct), arena,
@@ -446,6 +467,7 @@ Res ControlInit(Arena arena)
 void ControlFinish(Arena arena)
 {
   AVERT(Arena, arena);
+  AVER(arena->poolReady);
   arena->poolReady = FALSE;
   PoolFinish(MVPool(&arena->controlPoolStruct));
 }

From 7a324aa430cd87ea6e3a03ec5e3ff5fe55d171a7 Mon Sep 17 00:00:00 2001
From: Gareth Rees <gdr@ravenbrook.com>
Date: Mon, 10 Aug 2015 12:15:05 +0100
Subject: [PATCH 16/16] Start review checklist.

Copied from Perforce
 Change: 188101
 ServerID: perforce.ravenbrook.com
---
 mps/design/guide.review.txt        | 96 ++++++++++++++++++++++++++++++
 mps/design/index.txt               |  2 +
 mps/manual/source/design/index.rst |  1 +
 3 files changed, 99 insertions(+)
 create mode 100644 mps/design/guide.review.txt

diff --git a/mps/design/guide.review.txt b/mps/design/guide.review.txt
new file mode 100644
index 00000000000..b1298b26b90
--- /dev/null
+++ b/mps/design/guide.review.txt
@@ -0,0 +1,96 @@
+.. mode: -*- rst -*-
+
+Review checklist
+================
+
+:Tag: guide.review
+:Status: incomplete documentation
+:Author: Gareth Rees
+:Organization: Ravenbrook Limited
+:Date: 2015-08-10
+:Revision: $Id$
+:Copyright: See section `Copyright and License`_.
+:Index terms: pair: review; checklist
+
+
+Introduction
+------------
+
+_`.scope`: This document contains a list of checks to apply when
+reviewing code or other documents in the Memory Pool System.
+
+_`.readership`: This document is intended for reviewers.
+
+_`.example`: The "example" links are issues caused by a failure to
+apply the checklist item.
+
+_`.diff`: Some items in the checklist are particularly susceptible to
+being ignored if one reviews only via the version control diff. These
+items refer to this tag.
+
+
+Checklist
+---------
+
+_`.test`: If a new feature has been added to the code, is there a test
+case? Example: job003923_.
+
+.. _job003923: http://www.ravenbrook.com/project/mps/issue/job003923/
+
+_`.unwind`: If code has been updated in a function that unwinds its
+state in failure cases, have the failure cases been updated to
+correspond? Example: job003922_. See `.diff`_.
+
+.. _job003922: http://www.ravenbrook.com/project/mps/issue/job003922/
+
+
+
+Document History
+----------------
+
+2015-08-10  GDR_  Created.
+
+.. _GDR: http://www.ravenbrook.com/consultants/gdr/
+
+
+Copyright and License
+---------------------
+
+Copyright © 2015 Ravenbrook Limited. All rights reserved.
+<http://www.ravenbrook.com/>. This is an open source license. Contact
+Ravenbrook for commercial licensing options.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+#. Redistributions of source code must retain the above copyright
+   notice, this list of conditions and the following disclaimer.
+
+#. Redistributions in binary form must reproduce the above copyright
+   notice, this list of conditions and the following disclaimer in the
+   documentation and/or other materials provided with the distribution.
+
+#. Redistributions in any form must be accompanied by information on how
+   to obtain complete source code for this software and any
+   accompanying software that uses this software.  The source code must
+   either be included in the distribution or be available for no more than
+   the cost of distribution plus a nominal fee, and must be freely
+   redistributable under reasonable conditions.  For an executable file,
+   complete source code means the source code for all modules it contains.
+   It does not include source code for modules or files that typically
+   accompany the major components of the operating system on which the
+   executable file runs.
+
+**This software is provided by the copyright holders and contributors
+"as is" and any express or implied warranties, including, but not
+limited to, the implied warranties of merchantability, fitness for a
+particular purpose, or non-infringement, are disclaimed.  In no event
+shall the copyright holders and contributors be liable for any direct,
+indirect, incidental, special, exemplary, or consequential damages
+(including, but not limited to, procurement of substitute goods or
+services; loss of use, data, or profits; or business interruption)
+however caused and on any theory of liability, whether in contract,
+strict liability, or tort (including negligence or otherwise) arising in
+any way out of the use of this software, even if advised of the
+possibility of such damage.**
diff --git a/mps/design/index.txt b/mps/design/index.txt
index bc08dbc476e..9f4abf7b4f4 100644
--- a/mps/design/index.txt
+++ b/mps/design/index.txt
@@ -61,6 +61,7 @@ freelist_               Free list allocator
 guide.hex.trans_        Transliterating the alphabet into hexadecimal
 guide.impl.c.format_    Coding standard: conventions for the general format of C source code in the MPS
 guide.impl.c.naming_    Coding standard: conventions for internal names
+guide.review_           Review checklist
 interface-c_            C interface
 io_                     I/O subsystem
 keyword-arguments_      Keyword arguments
@@ -138,6 +139,7 @@ writef_                 The WriteF function
 .. _guide.hex.trans: guide.hex.trans
 .. _guide.impl.c.format: guide.impl.c.format
 .. _guide.impl.c.naming: guide.impl.c.naming
+.. _guide.review: guide.review
 .. _interface-c: interface-c
 .. _io: io
 .. _keyword-arguments: keyword-arguments
diff --git a/mps/manual/source/design/index.rst b/mps/manual/source/design/index.rst
index e6b2adbd61a..907c9986bb1 100644
--- a/mps/manual/source/design/index.rst
+++ b/mps/manual/source/design/index.rst
@@ -17,6 +17,7 @@ Design
     guide.hex.trans
     guide.impl.c.format
     guide.impl.c.naming
+    guide.review
     interface-c
     keyword-arguments
     land