diff --git a/cmd/vgpud/main.c b/cmd/vgpud/main.c index 516b11d..b4e2752 100644 --- a/cmd/vgpud/main.c +++ b/cmd/vgpud/main.c @@ -433,9 +433,16 @@ bring_up(void) struct pciuser_dma d; d.size = fb_bytes; if (ioctl(pcifd, PCIIOC_DMA, &d) < 0) { + // The kernel has already printed how much is free and how big the + // largest piece is, which is the part that says whether this is a full + // arena or a fragmented one. All this adds is what was being asked + // for, and the resolution that asked for it -- because the usual cause + // is a mode nobody realised was this expensive. puts("vgpud: wanted "); putn(fb_bytes / 1024); - puts(" KB of DMA memory for the framebuffer\n"); - die("not enough DMA memory -- see DMA_ARENA_BYTES"); + puts(" KB of DMA memory for a "); putn(width); puts("x"); putn(height); + puts(" framebuffer\n"); + puts("vgpud: `schdbg dma` shows the arena; a smaller mode needs less\n"); + die("not enough contiguous DMA memory"); } fb_uaddr = d.uaddr; fb_paddr = d.paddr; diff --git a/docs/architecture/devices.md b/docs/architecture/devices.md index 23facbd..3adb72f 100644 --- a/docs/architecture/devices.md +++ b/docs/architecture/devices.md @@ -94,6 +94,23 @@ first minute". which is precisely what it cannot work out for itself, and the memory comes from an arena reserved at boot before the general allocator gets anything. +Everything in that arena has to be **contiguous**, which is the whole reason it +exists — so the number that matters is not how much is free but how large the +biggest piece is. `dma_alloc()` is therefore best fit rather than first fit: a +64 KB ring taken from the middle of a 16 MB arena leaves two 8 MB extents, and +a framebuffer wanting 9 MB then cannot be had on an arena three quarters empty. +Best fit puts each request in the tightest extent that holds it and leaves the +large ones whole for the requests with no alternative. + +A failed allocation reports both numbers for the same reason, because they +point at different fixes — a fragmented arena is cured by whoever holds the +block in the middle letting go, a full one by `DMA_ARENA_BYTES`: + +``` +dma_alloc: no room for 9216000 bytes -- 12591104 free, + largest contiguous piece 8495104 (`schdbg dma` for the map) +``` + **Interrupts arrive by `read()`**, the way Linux's uio does it and for the same reason: a descriptor composes with `poll`, and a driver that stops reading falls behind rather than losing events. The kernel masks the line when the interrupt diff --git a/include/dma.h b/include/dma.h index f15c3e0..a410018 100644 --- a/include/dma.h +++ b/include/dma.h @@ -51,4 +51,9 @@ void dma_free(void *p, uint64 size); addr_t dma_phys(const void *p); uint64 dma_free_bytes(void); + +// The biggest single block still available. When an allocation fails, the +// difference between this and dma_free_bytes() is the difference between a +// full arena and a fragmented one -- see the message in dma_alloc(). +uint64 dma_largest_bytes(void); uint64 dma_total_bytes(void); diff --git a/kernel/core/dma.c b/kernel/core/dma.c index 82a457c..dd9c153 100644 --- a/kernel/core/dma.c +++ b/kernel/core/dma.c @@ -187,15 +187,57 @@ dma_alloc(uint64 size, uint64 align) acquire(&arena.lock); - for(i = 0; i < arena.n; i++){ + // Best fit, not first fit. + // + // Everything in this arena has to be *contiguous* -- that is the entire + // reason it exists -- so the number that matters is not how much is free but + // how big the largest piece is. First fit destroys that number cheaply: a + // 64 KB ring, allocated at boot from the front of a 16 MB arena, leaves one + // 16 MB extent alone; allocated from the middle it leaves two of 8 MB, and a + // framebuffer that needed 9 MB now cannot be had on an arena three quarters + // empty. + // + // That is not hypothetical -- it is what the arena looked like in practice: + // + // 16384 KB total, 12295 KB free in 3 extent(s), 4088 KB out + // 0 ... 4091904 <- too small for a 4000 KB framebuffer, by 4 KB + // 1 ... 3584 + // 2 ... 8495104 + // + // Best fit puts each request in the tightest extent that will hold it, which + // leaves the large ones whole for the requests that have no alternative. It + // is O(n) over a table that holds a handful of entries, so the cost is + // nothing, and it cannot fragment *worse* than first fit for any request. + { + int best = -1; + uint64 best_waste = 0; + + for(i = 0; i < arena.n; i++){ + addr_t base = arena.free[i].base; + uint64 len = arena.free[i].len; + uint64 lead = roundup(base, align) - base; + uint64 waste; + + if(lead + size > len) + continue; + waste = len - size - lead; + if(best < 0 || waste < best_waste){ + best = i; + best_waste = waste; + if(waste == 0) + break; // an exact fit cannot be improved on + } + } + i = best; + } + + if(i >= 0){ addr_t base = arena.free[i].base; uint64 len = arena.free[i].len; uint64 lead; start = roundup(base, align); lead = start - base; - if(lead + size > len) - continue; // Three cases: the request consumes the extent, sits at its head, sits // at its tail, or sits in the middle and splits it in two. @@ -213,7 +255,11 @@ dma_alloc(uint64 size, uint64 align) // No room to record the tail. Give the whole extent to the caller // rather than lose track of where the tail was. arena.free[i].len = len; - continue; + release(&arena.lock); + cprintf("dma_alloc: extent table full; %d bytes handed out with it\n", + (int)(len - size)); + memset((void*)start, 0, size); + return (void*)start; } } @@ -223,8 +269,19 @@ dma_alloc(uint64 size, uint64 align) } release(&arena.lock); - cprintf("dma_alloc: out of DMA memory (wanted %d bytes, %d free)\n", - (int)size, (int)dma_free_bytes()); + + // Both numbers, because they say different things and only one of them is + // usually the problem. "12 MB free" and "the biggest piece is 3 MB" is not a + // full arena, it is a fragmented one, and the fix for the two is not the + // same: a fragmented arena is cured by whoever is holding the block in the + // middle letting go, and a full one is cured by DMA_ARENA_BYTES. + // + // This distinction is invisible from the old message, which reported only + // the total and so read as "out of memory" on a machine with three quarters + // of the arena free. + cprintf("dma_alloc: no room for %d bytes -- %d free, " + "largest contiguous piece %d (`schdbg dma` for the map)\n", + (int)size, (int)dma_free_bytes(), (int)dma_largest_bytes()); return 0; } @@ -282,6 +339,24 @@ dma_free_bytes(void) return n; } +// The largest single allocation this arena could still satisfy. +// +// Not the same as dma_free_bytes(), and the gap between them is the whole +// story when an allocation fails on an arena that is mostly free. +uint64 +dma_largest_bytes(void) +{ + uint64 best = 0; + int i; + + acquire(&arena.lock); + for(i = 0; i < arena.n; i++) + if(arena.free[i].len > best) + best = arena.free[i].len; + release(&arena.lock); + return best; +} + uint64 dma_total_bytes(void) {