diff --git a/docs/building.md b/docs/building.md index 0e4f472b..29d4aa2b 100644 --- a/docs/building.md +++ b/docs/building.md @@ -697,7 +697,8 @@ does not either when a range in flight in its region ends during that test. A fe of its range: freezes, thaws, arms, releases and copies of other ranges return at once meanwhile, also while a thaw fetches the armed ranges of its own regions, two ranges fetch at once, and more touching threads than the flight table holds all end. The touches of a range in flight wait for it and run again, a write among them without releasing the -region's other ranges; thaws, mapping changes, releases of its buffer, arms and freezes see it. The same with +region's other ranges, also when the flight ended while the write waited for the freeze lock; thaws, mapping changes, +releases of its buffer, arms and freezes see it. The same with `wb_lazy_chunk = 1`, and with `wb_lazy_unlocked = 0` the fetch holds the freeze lock again, a thaw's too, and a release fetches one range at a time (the table of ranges in flight stays: a touch of one still waits for it alone). The "mem: lazy write-back" lines give the longest fetch, the longest wait of a fault for the freeze lock and the diff --git a/runtime/memory.c b/runtime/memory.c index ff0a95df..1fc5aba9 100644 --- a/runtime/memory.c +++ b/runtime/memory.c @@ -266,15 +266,17 @@ static volatile uint32_t lz_fetch_ms; // plat_now_ms() | 1 at the end of th // - an arm leaves its pages alone (mem_lazy_arm), a freeze its region (lzr), and mem_lazy_touches counts it. // Every segment that leaves the list with inaccessible pages becomes an entry in the same lz_lock hold, and an entry // ends only once its pages are read-write; lists and entries never overlap. So a fault that finds its address in -// neither, holding frz_lock (no arm runs then), has an address that is not ours or no longer inaccessible. An entry -// starts under frz_lock and lz_lock and ends under lz_lock alone, so a holder of frz_lock may wait for one: its fetch -// takes the queue lock, and no holder of the queue lock takes frz_lock. Every store to lzr is made under lz_lock, from -// the list, the entries and the region an arm is protecting (lz_arm_r). Fetches of different ranges run at once (the -// back end has a slot for each, vk_memory.c lazy_fetch). NX2NAVI_WB_LAZY_UNLOCKED=0 brings back 0.10.1's locking: frz_lock -// is kept from the moment a fault or a release takes a range out until its entry ends, a release takes one segment at a -// time, mem_thaw releases under the lock, and vk_memory.c mirror_evict and hot_check release their victims under -// pool_lock, so one fetch runs at a time. The entries stay with it: a touch of a range in flight waits for that entry -// and runs again, the hold is set at the take-out, and mem_lazy_touches counts the range. +// neither, holding frz_lock (no arm runs then), has an address that is not ours or no longer inaccessible: the page's +// state, asked under that lock, decides whether the access runs again (lazy_fault), and an arm after it only has the +// access fault here once more. An entry starts under frz_lock and lz_lock and ends under lz_lock alone, so a holder of +// frz_lock may wait for one: its fetch takes the queue lock, and no holder of the queue lock takes frz_lock. Every +// store to lzr is made under lz_lock, from the list, the entries and the region an arm is protecting (lz_arm_r). +// Fetches of different ranges run at once (the back end has a slot for each, vk_memory.c lazy_fetch). +// NX2NAVI_WB_LAZY_UNLOCKED=0 brings back 0.10.1's locking: frz_lock is kept from the moment a fault or a release takes +// a range out until its entry ends, a release takes one segment at a time, mem_thaw releases under the lock, and +// vk_memory.c mirror_evict and hot_check release their victims under pool_lock, so one fetch runs at a time. The +// entries stay with it: a touch of a range in flight waits for that entry and runs again, the hold is set at the +// take-out, and mem_lazy_touches counts the range. #define LZ_FLY 64 // more than the threads that touch armed pages at once; when full, a touch waits for room #define LZ_BATCH 16 // segments a release takes out together and fetches in one call (1 with the setting at 0) typedef struct { uint64_t lo, hi, h0, bytes; uint32_t seq, tid; uint8_t used; } LzFly; // bytes: to fetch, 0 none @@ -814,8 +816,9 @@ void mem_lazy_window(int open) { plat_mutex_unlock(&lz_lock); } -// An access fault at a: 1 when a was armed or in flight and its pages are read-write again (the access runs again), 0 -// when a is none of the lazy write-back's. +// An access fault at a: 1 when a was armed or in flight and its pages are read-write again, or when its page allows the +// access by now (another thread gave it back): the access runs again. 0 when a is none of the lazy write-back's and its +// page does not allow the access (a write to a frozen page goes on to thaw_fault). static int lazy_fault(uint64_t a, int write) { if (a >= RT_ADDRESS_LIMIT || !__atomic_load_n(&lzr[a >> FRZ_SHIFT], __ATOMIC_ACQUIRE)) return 0; uint32_t me = plat_thread_id(); @@ -834,11 +837,23 @@ static int lazy_fault(uint64_t a, int write) { plat_mutex_lock(&lz_lock); // the window's test and the segment's leaving the list in one step int i = 0; while (i < lz_n && !(a >= lzs[i].lo && a < lzs[i].hi)) i++; - if (i == lz_n) { // in flight since the look above; else not ours, or given back (no arm runs under frz_lock) - plat_mutex_unlock(&frz_lock); - int joined = lazy_fly_join_locked(a); + if (i == lz_n) { + if (lazy_fly_find_locked(a, a + 1, 0) >= 0) { // in flight since the look above: wait for it alone + plat_mutex_unlock(&frz_lock); + lazy_fly_join_locked(a); + plat_mutex_unlock(&lz_lock); + return 1; + } + // Not ours, or given back since the fault: the page's state decides, asked under frz_lock alone (no system + // call under lz_lock), so that no arm makes the page inaccessible between the answer and the retry. A write + // to a page a flight gave back runs again here instead of going on to thaw_fault, whose thaw would release + // every armed range of the region; and a read is not passed to the crash handler because an arm (after a + // release that sets no hold, mem_lazy_skip's give-back) made the page inaccessible again once this returned. plat_mutex_unlock(&lz_lock); - return joined; + uint64_t pg = a & ~(PAGE - 1), end; + int st = plat_mem_state(pg, pg + PAGE, &end); + plat_mutex_unlock(&frz_lock); + return write ? st == PLAT_PROT_RW : st >= PLAT_PROT_READ; } uint32_t w = __atomic_load_n(&lz_win_tid, __ATOMIC_ACQUIRE); if (w && w != me && tries < 400) { // up to 20 ms: after the hand-over the fetch has that part's writes diff --git a/tests/lazy_fetch_check.c b/tests/lazy_fetch_check.c index 8c8b7d6c..6abbe390 100644 --- a/tests/lazy_fetch_check.c +++ b/tests/lazy_fetch_check.c @@ -268,6 +268,30 @@ int main(int argc, char **argv) { CHECK(armed(s2, K64), "the region's other range was released"); CHECK(lz_other == 0, "%llu other releases", (unsigned long long)lz_other); CHECK(b.took < 250 && b.got == 0x77, "the write waited %.1f ms and read back %#x", b.took, b.got); + } else if (!strcmp(what, "late_write")) { + // B writes a range whose flight starts and ends while B waits for frz_lock in lazy_fault (this thread holds the + // lock and runs a release's flight under it, as a thaw does): B finds the page given back and writes again, and + // the region's other range stays armed (B went on to thaw_fault before, whose thaw released it) + uint64_t R = heap + 128 * M, s1 = R, s2 = R + M; + CHECK(arm_buf(s1, K64, 1) == K64 && arm_buf(s2, K64, 1) == K64, "arms"); + stale(s1, K64, 1), stale(s2, K64, 1); + fetch_ns = 20000000ull; + plat_mutex_lock(&frz_lock); + Touch b; + void *tb = touch(&b, s1 + 0x20, 1); + plat_wait_ns(50000000); // B waits for frz_lock in lazy_fault, past its look for a range in flight + int k[LZ_BATCH]; + LzSeg sg[LZ_BATCH]; + uint64_t rg[1][3] = {{s1, s1 + K64, 0}}; + plat_mutex_lock(&lz_lock); + int nt = lazy_take_batch_locked((const uint64_t (*)[3])rg, 1, k, sg); + plat_mutex_unlock(&lz_lock); + if (nt > 0) lazy_fly_run(k, sg, nt); + plat_mutex_unlock(&frz_lock); + plat_thread_join(tb); + CHECK(nt == 1 && b.got == 0x77, "%d taken out, the write read back %#x", nt, b.got); + CHECK(armed(s2, K64) && lz_other == 0, "the region's other range was released (%llu other releases)", + (unsigned long long)lz_other); } else if (!strcmp(what, "concurrent")) { // two threads touch two ranges at once: their fetches run at the same time uint64_t s1 = heap + 40 * M, s2 = heap + 48 * M; diff --git a/tests/lazy_fetch_check.py b/tests/lazy_fetch_check.py index 89922ef1..9c4d87c6 100644 --- a/tests/lazy_fetch_check.py +++ b/tests/lazy_fetch_check.py @@ -27,6 +27,7 @@ SCENARIOS = { 'no_stall': ('no_stall', {}), 'join': ('join', {}), 'loser': ('loser', {}), + 'late_write': ('late_write', {}), 'concurrent': ('concurrent', {}), 'thaw': ('thaw', {}), 'thaw_unlocked': ('thaw_unlocked', {}),