dma: best fit, and say which number is the problem

The arena hands out contiguous memory -- that is the only reason it exists --
so the number that matters is not how much is free but how big the largest
piece is. First fit spends that number cheaply: 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.

dma_alloc() is best fit now: the tightest extent that holds the request, which
leaves the large ones whole for the requests that have no alternative. O(n)
over a table with a handful of entries, and it cannot fragment worse than first
fit for any request.

And the failure says both numbers, because they point at different fixes and
the old message only had one of them:

    dma_alloc: no room for 9216000 bytes -- 12591104 free,
               largest contiguous piece 8495104 (`schdbg dma` for the map)

"12 MB free" alone reads as out-of-memory on a machine that has plenty; the gap
between the two is the whole story. A fragmented arena is cured by whoever is
holding the block in the middle letting go, a full one by DMA_ARENA_BYTES.

Also fixes a real bug found while reading it: when the extent table was full,
the tail-split path did `continue` while still holding the lock and having
already committed the extent to the caller -- so it carried on scanning, and
could return a second block or fall through to the failure path having handed
one out. It now returns the block it committed, and says so.

**This does not close the report it came from.** vgpud failing to get a
framebuffer was not reproduced here: at 1280x800 the arena has 8.1 MB
contiguous against a 4 MB request. What is fixed is that the next occurrence
explains itself instead of saying "not enough DMA memory" on an empty arena.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQ7Qx8E1q1M1bbCq8QLHS2

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");
+        puts(" KB of DMA memory for a "); putn(width); puts("x"); putn(height);
-        die("not enough DMA memory -- see DMA_ARENA_BYTES");
+        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;

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

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);

kernel/core/dma.c

@@ -187,15 +187,57 @@ dma_alloc(uint64 size, uint64 align)
 
   acquire(&arena.lock);
 
+  // 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)
 {