From 4fc8971802166e8ff50bcb0e61e07b715a23c17e Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Mon, 17 Feb 2014 16:45:12 +0000 Subject: [PATCH] Minor updates in response to review. see https://info.ravenbrook.com/mail/2014/02/17/16-27-18/0/ Copied from Perforce Change: 184354 ServerID: perforce.ravenbrook.com --- mps/code/arenacl.c | 5 ++--- mps/code/arenavm.c | 24 ++++++++++++------------ mps/code/sa.c | 5 +++-- mps/code/sa.h | 2 +- mps/code/tract.c | 26 ++++++++++++++------------ mps/code/tract.h | 38 ++++++++++++-------------------------- mps/design/arena.txt | 6 ++++-- mps/design/arenavm.txt | 11 ++++------- 8 files changed, 52 insertions(+), 65 deletions(-) diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c index 6fbf91e8a73..473ac37a40e 100644 --- a/mps/code/arenacl.c +++ b/mps/code/arenacl.c @@ -392,7 +392,7 @@ static Res chunkAlloc(Addr *baseReturn, Tract *baseTractReturn, clChunk->freePages -= pages; *baseReturn = PageIndexBase(chunk, baseIndex); - *baseTractReturn = PageTract(&chunk->pageTable[baseIndex]); + *baseTractReturn = PageTract(ChunkPage(chunk, baseIndex)); return ResOK; } @@ -471,8 +471,7 @@ static void ClientFree(Addr base, Size size, Pool pool) AVER(limitIndex <= chunk->pages); for(pi = baseIndex; pi < limitIndex; pi++) { - Page page = &chunk->pageTable[pi]; - Tract tract = PageTract(page); + Tract tract = PageTract(ChunkPage(chunk, pi)); AVER(TractPool(tract) == pool); TractFinish(tract); diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c index f0a2c48b620..edddd7cc658 100644 --- a/mps/code/arenavm.c +++ b/mps/code/arenavm.c @@ -105,15 +105,15 @@ static Bool VMChunkCheck(VMChunk vmchunk) CHECKL(VMCheck(vmchunk->vm)); CHECKL(VMAlign(vmchunk->vm) == ChunkPageSize(chunk)); CHECKL(vmchunk->overheadMappedLimit <= (Addr)chunk->pageTable); - /* check pageTableMapped table */ - /* FIXME: Check sa's tables */ CHECKD(SparseArray, &vmchunk->pages); -#if 0 - CHECKL(vmchunk->pageTableMapped != NULL); - CHECKL((Addr)vmchunk->pageTableMapped >= chunk->base); - CHECKL(AddrAdd((Addr)vmchunk->pageTableMapped, BTSize(chunk->pageTablePages)) - <= vmchunk->overheadMappedLimit); -#endif + /* SparseArrayCheck is agnostic about where the BTs live, so VMChunkCheck + makes sure they're where they're expected to be (in the chunk). */ + CHECKL(chunk->base < (Addr)vmchunk->pages.mapped); + CHECKL(AddrAdd(vmchunk->pages.mapped, BTSize(chunk->pages)) <= + vmchunk->overheadMappedLimit); + CHECKL(chunk->base < (Addr)vmchunk->pages.pages); + CHECKL(AddrAdd(vmchunk->pages.pages, BTSize(chunk->pageTablePages)) <= + vmchunk->overheadMappedLimit); /* .improve.check-table: Could check the consistency of the tables. */ return TRUE; @@ -1039,7 +1039,7 @@ static unsigned pageState(VMChunk vmChunk, Index pi) { Chunk chunk = VMChunk2Chunk(vmChunk); if (SparseArrayIsMapped(&vmChunk->pages, pi)) - return PageState(&chunk->pageTable[pi]); + return PageState(ChunkPage(chunk, pi)); return PageStateFREE; } @@ -1053,7 +1053,7 @@ static void sparePageRelease(VMChunk vmChunk, Index pi) { Chunk chunk = VMChunk2Chunk(vmChunk); Arena arena = ChunkArena(chunk); - Page page = &chunk->pageTable[pi]; + Page page = ChunkPage(chunk, pi); AVER(PageState(page) == PageStateSPARE); AVER(arena->spareCommitted >= ChunkPageSize(chunk)); @@ -1214,7 +1214,7 @@ static Res vmAllocComm(Addr *baseReturn, Tract *baseTractReturn, } base = PageIndexBase(chunk, baseIndex); - baseTract = PageTract(&chunk->pageTable[baseIndex]); + baseTract = PageTract(ChunkPage(chunk, baseIndex)); limit = AddrAdd(base, size); zones = ZoneSetOfRange(arena, base, limit); @@ -1410,7 +1410,7 @@ static void VMFree(Addr base, Size size, Pool pool) /* loop from pageBase to pageLimit-1 inclusive */ /* Finish each Tract found, then convert them to spare pages. */ for(pi = piBase; pi < piLimit; ++pi) { - Page page = &chunk->pageTable[pi]; + Page page = ChunkPage(chunk, pi); Tract tract = PageTract(page); AVER(TractPool(tract) == pool); diff --git a/mps/code/sa.c b/mps/code/sa.c index bd2d91fe46d..909ed4bf1dd 100644 --- a/mps/code/sa.c +++ b/mps/code/sa.c @@ -44,10 +44,11 @@ void SparseArrayFinish(SparseArray sa) Bool SparseArrayCheck(SparseArray sa) { CHECKL(sa != NULL); - CHECKL(sa->sig == SparseArraySig); + CHECKS(SparseArray, sa); CHECKL(sa->base != NULL); CHECKL(sa->elementSize >= 1); - CHECKL(VMCheck(sa->vm)); /* TODO: CHECKD(VM, sa->vm); */ + /* TODO: CHECKD(VM, sa->vm); once VMStruct becomes visible */ + CHECKL(VMCheck(sa->vm)); CHECKL(sa->elementSize <= VMAlign(sa->vm)); CHECKL(sa->length > 0); CHECKL(BTCheck(sa->mapped)); diff --git a/mps/code/sa.h b/mps/code/sa.h index 40aa133bbf2..c56ba4b0508 100644 --- a/mps/code/sa.h +++ b/mps/code/sa.h @@ -21,7 +21,7 @@ typedef struct SparseArrayStruct *SparseArray; -#define SparseArraySig ((Sig)0x5195BA66) /* SIGnature SParse ARRAy */ +#define SparseArraySig ((Sig)0x5195BA66) /* SIGnature SParse ARRay */ typedef struct SparseArrayStruct { Sig sig; diff --git a/mps/code/tract.c b/mps/code/tract.c index 5e3cad6875f..92e16f6fbca 100644 --- a/mps/code/tract.c +++ b/mps/code/tract.c @@ -446,8 +446,7 @@ Bool TractOfAddr(Tract *tractReturn, Arena arena, Addr addr) /* either the page is free or it is */ /* part of the arena tables (see .ullagepages). */ if (BTGet(chunk->allocTable, i)) { - Page page = &chunk->pageTable[i]; - *tractReturn = PageTract(page); + *tractReturn = PageTract(ChunkPage(chunk, i)); return TRUE; } @@ -494,15 +493,15 @@ static Bool tractSearchInChunk(Tract *tractReturn, Chunk chunk, Index i) AVER_CRITICAL(chunk->allocBase <= i); AVER_CRITICAL(i <= chunk->pages); - while(i < chunk->pages - && !(BTGet(chunk->allocTable, i) - && PageIsAllocated(&chunk->pageTable[i]))) { + while (i < chunk->pages + && !(BTGet(chunk->allocTable, i) + && PageIsAllocated(ChunkPage(chunk, i)))) { ++i; } if (i == chunk->pages) return FALSE; AVER(i < chunk->pages); - *tractReturn = PageTract(&chunk->pageTable[i]); + *tractReturn = PageTract(ChunkPage(chunk, i)); return TRUE; } @@ -602,7 +601,7 @@ void PageAlloc(Chunk chunk, Index pi, Pool pool) AVER(!BTGet(chunk->allocTable, pi)); AVERT(Pool, pool); - page = &chunk->pageTable[pi]; + page = ChunkPage(chunk, pi); tract = PageTract(page); base = PageIndexBase(chunk, pi); BTSet(chunk->allocTable, pi); @@ -614,15 +613,18 @@ void PageAlloc(Chunk chunk, Index pi, Pool pool) void PageInit(Chunk chunk, Index pi) { + Page page; + AVERT(Chunk, chunk); AVER(pi < chunk->pages); + + page = ChunkPage(chunk, pi); BTRes(chunk->allocTable, pi); - PageSetPool(&chunk->pageTable[pi], NULL); - PageSetType(&chunk->pageTable[pi], PageStateFREE); - RingInit(PageSpareRing(&chunk->pageTable[pi])); - RingInit(PageFreeRing(&chunk->pageTable[pi])); - return; + PageSetPool(page, NULL); + PageSetType(page, PageStateFREE); + RingInit(PageSpareRing(page)); + RingInit(PageFreeRing(page)); } diff --git a/mps/code/tract.h b/mps/code/tract.h index 63892b1872b..24d8a2af86d 100644 --- a/mps/code/tract.h +++ b/mps/code/tract.h @@ -15,31 +15,16 @@ /* Page states * - * .states: Pages (hence PageUnions that describe them) can be in - * one of 3 states: - * allocated (to a pool as tracts) - * allocated pages are mapped - * BTGet(allocTable, i) == 1 - * PageState() == PageStateALLOC - * PagePool()->pool == pool - * spare - * these pages are mapped - * BTGet(allocTable, i) == 0 - * PageState() == PageStateSPARE - * PagePool() == NULL - * free - * these pages are not mapped - * BTGet(allocTable, i) == 0 - * PTE may itself be unmapped, but when it is (use pageTableMapped - * to determine whether page occupied by page table is mapped): - * PagePool() == NULL - * PageState() == PageStateFREE + * .states: The first word of the page descriptor contains a pointer to + * the page's owning pool if the page is allocated. The bottom two bits + * indicate the page state. Note that the page descriptor itself may + * not be mapped since it is stored in a SparseArray. */ -#define PageStateALLOC 0 -#define PageStateSPARE 1 -#define PageStateFREE 2 -#define PageStateWIDTH 2 /* bitfield width */ +#define PageStateALLOC 0 /* allocated to a pool as a tract */ +#define PageStateSPARE 1 /* free but mapped to backing store */ +#define PageStateFREE 2 /* free and unmapped (address space only) */ +#define PageStateWIDTH 2 /* bitfield width */ typedef union PagePoolUnion { unsigned state : PageStateWIDTH; /* see .states */ @@ -168,11 +153,12 @@ typedef struct ChunkStruct { } ChunkStruct; -#define ChunkArena(chunk) ((chunk)->arena) -#define ChunkPageSize(chunk) ((chunk)->pageSize) -#define ChunkPageShift(chunk) ((chunk)->pageShift) +#define ChunkArena(chunk) RVALUE((chunk)->arena) +#define ChunkPageSize(chunk) RVALUE((chunk)->pageSize) +#define ChunkPageShift(chunk) RVALUE((chunk)->pageShift) #define ChunkPagesToSize(chunk, pages) ((Size)(pages) << (chunk)->pageShift) #define ChunkSizeToPages(chunk, size) ((Count)((size) >> (chunk)->pageShift)) +#define ChunkPage(chunk, pi) (&(chunk)->pageTable[pi]) extern Bool ChunkCheck(Chunk chunk); extern Res ChunkInit(Chunk chunk, Arena arena, diff --git a/mps/design/arena.txt b/mps/design/arena.txt index 65b41f74ccd..a1fae81ce5e 100644 --- a/mps/design/arena.txt +++ b/mps/design/arena.txt @@ -233,14 +233,14 @@ associating their own data with each allocation grain. _`.tract.structure`: The tract structure definition looks like this:: typedef struct TractStruct { /* Tract structure */ - Pool pool; /* MUST BE FIRST (design.mps.arena.tract.field.pool) */ + PagePoolUnion pool; /* MUST BE FIRST (design.mps.arena.tract.field.pool) */ void *p; /* pointer for use of owning pool */ Addr base; /* Base address of the tract */ TraceSet white : TRACE_MAX; /* traces for which tract is white */ unsigned int hasSeg : 1; /* does tract have a seg in p? */ } TractStruct; -_`.tract.field.pool`: The pool field indicates to which pool the tract +_`.tract.field.pool`: The pool.pool field indicates to which pool the tract has been allocated (`.req.fun.trans.pool`_). Tracts are only valid when they are allocated to pools. When tracts are not allocated to pools, arena classes are free to reuse tract objects in undefined @@ -546,6 +546,8 @@ Document History - 2013-03-11 GDR_ Converted to reStructuredText. +- 2014-02-17 RB_ Updated first field of tract structure. + .. _RB: http://www.ravenbrook.com/consultants/rb/ .. _GDR: http://www.ravenbrook.com/consultants/gdr/ diff --git a/mps/design/arenavm.txt b/mps/design/arenavm.txt index ffe97432aa1..92b24d8234d 100644 --- a/mps/design/arenavm.txt +++ b/mps/design/arenavm.txt @@ -167,13 +167,7 @@ the base address in the VM. Namely: - base address to index: ``index = (base-address - arena-base) / page-size`` _`.table.page.partial`: The table is partially mapped on an -"as-needed" basis. The function ``unusedTablePages()`` identifies -entirely unused pages occupied by the page table itself (that is, -those pages of the page table which are occupied by ``PageStruct`` -objects which all describe free pages). Tract allocation and freeing -use this function to map and unmap the page table with no hysteresis. -(There is restriction on the parameters you may pass to -``unusedTablePages()``.) +"as-needed" basis using the SparseArray abstract type. _`.table.page.tract`: Each page table entry contains a tract, which is only valid if it is allocated to a pool. If it is not allocated to a @@ -215,6 +209,9 @@ Document History - 2013-05-24 GDR_ Converted to reStructuredText. +- 2014-02-17 RB_ Updated to note use of SparseArray rather than direct +management of page table mapping. + .. _RB: http://www.ravenbrook.com/consultants/rb/ .. _GDR: http://www.ravenbrook.com/consultants/gdr/