From c98bdf0019b54ea2593fa5978140c3cef6a64bdf Mon Sep 17 00:00:00 2001 From: Nexoniarz Date: Tue, 22 Sep 2026 09:31:55 +0200 Subject: [PATCH] kmem: a slab allocator, and what the fixed tables actually cost Gate 0's last structural row. kernel/core/kmem.c: slabs are pages from kalloc() with a header at the front, and free objects hold the free list in their own first eight bytes. That last detail is the whole design. **An object carries no header and freeing performs no search**: the page base of any object *is* its slab, and the slab names its cache. It is also exactly why an object cannot exceed a page, which matters below. Two interfaces. kmem_cache_create/alloc/free for a subsystem with one kind of thing -- it gets its own line in /proc/slab, which is most of why naming it is worth the argument -- and kmalloc/kmfree over power-of-two classes from 16 to 2048 for code with no single type to build a cache around. Each cache keeps partial and full lists; a slab that empties goes straight back to kalloc() rather than being kept, so /proc/slab never overstates what is held. `make kmem-test` asserts the bookkeeping on the build machine. Same arrangement as schfs-objtest, and for the same reason: slab lists, free-list threading, the page-to-cache lookup and whether pages actually come back are all things that can be wrong without a kernel to be wrong in. 26 checks, including one that a permissive allocator cannot pass -- write a distinct pattern through 512 objects and read them all back, because two objects sharing storage shows up there and nowhere else. One bug the test found immediately: kmem_max_obj() returned the raw page remainder, which kmem_cache_create() then rounded *up* past its own limit. The largest legal size was reported as illegal. **It does not do what the register said it would.** The claim was that every fixed table exists to work around kalloc. Measured: struct proc 10656 NPROC 128 larger than a page struct socket 4928 NSOCK 32 larger than a page struct buf 4272 NBUF 256 larger than a page struct inode 280 NINODE 200 a cache, bounded on purpose struct device 120 NDEVICE 32 3.8 KB, never hit struct file 48 NFILE 512 24 KB, never hit The three largest cannot go in a page-backed slab whatever it does. NINODE is a cache and is meant to be bounded. NFILE and NDEVICE are small enough that their tables are rounding errors and their ceilings have never been reached. Freeing the big ones needs a contiguous multi-page allocator, which is a different thing and is not written. That is now in the register and in docs/architecture/kernel.md rather than left as a claim nobody checked. What it does fix is real, and was measured rather than assumed: - **A pipe is 640 bytes and came from a whole 4096-byte page.** Every `|` in a shell command burned three and a half kilobytes it never touched. Six pipes per page now, and /proc/slab shows the page going back when they close. - **procread() built its answer in a 2048-byte array on the kernel stack**, under a comment that had warned about exactly that since it was written -- it stayed a local because a whole page for 2 KB felt worse, and kmalloc is the answer that was missing. Its frame is 64 bytes now instead of ~2100. - procfs's other generator took a page from kalloc for the same 2 KB. Verified on a boot: /proc/slab tracks pipe and kmalloc-2048 through real pipelines, allocs and frees balance, and $(), ps, time, readlink, sigtest and rename are all still green with no traps. Co-Authored-By: Claude Opus 5 --- .gitignore | 6 + Makefile | 17 +- docs/architecture/kernel.md | 48 ++++++ docs/plan/1.0.md | 33 +++- include/kmem.h | 58 +++++++ kernel/core/kmem.c | 318 ++++++++++++++++++++++++++++++++++++ kernel/dev/procfs.c | 23 ++- kernel/fs/pipe.c | 16 +- kernel/fs/procfs.c | 12 +- tools/kmemtest.c | 136 +++++++++++++++ 10 files changed, 651 insertions(+), 16 deletions(-) create mode 100644 include/kmem.h create mode 100644 kernel/core/kmem.c create mode 100644 tools/kmemtest.c diff --git a/.gitignore b/.gitignore index 32de507..3a26800 100644 --- a/.gitignore +++ b/.gitignore @@ -48,3 +48,9 @@ docs/Website/releases/ # ours; a .ko here would be someone else's GPL code in an Apache-2.0 repo. *.ko *.ko.xz + +# Core dumps from host-side test harnesses (tools/*test.c run on the build +# machine). One of these got committed once; the pattern is cheaper than +# remembering. +core +core.* diff --git a/Makefile b/Makefile index fcb116d..def49fe 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ KERN_ARCH = $(addprefix $(BUILD)/kernel/arch/x86_64/, \ KERN_CORE = $(addprefix $(BUILD)/kernel/core/, \ main.o kalloc.o dma.o spinlock.o sleeplock.o string.o console.o \ - panic.o uaccess.o qr.o time.o ksym.o klog.o sbuf.o) + panic.o uaccess.o qr.o time.o ksym.o klog.o sbuf.o kmem.o) KERN_DEV = $(addprefix $(BUILD)/kernel/dev/, \ bio.o cdev.o device.o disk.o fb.o ide.o input.o irq.o kbd.o keymap.o mouse.o \ @@ -1015,6 +1015,19 @@ $(SCHFS_OBJT): tools/schfs_objtest.c $(SCHFS_SRC) schfs-objtest: $(SCHFS_OBJT) $(SCHFS_OBJT) +# The slab allocator, on the host. Same argument as schfs-objtest above: +# kernel/core/kmem.c is written to compile on the build machine, so its +# bookkeeping -- slab lists, the free list threaded through free objects, the +# page-to-cache lookup, and whether pages actually come back -- is asserted in +# a second rather than in a boot. +KMEMTEST = $(BUILD)/tools/kmemtest$(EXE) +$(KMEMTEST): tools/kmemtest.c kernel/core/kmem.c include/kmem.h + @mkdir -p $(BUILD)/tools + $(HOSTCC) $(HOSTCFLAGS) -I include -o $@ tools/kmemtest.c + +kmem-test: $(KMEMTEST) + $(KMEMTEST) + # The image writer and checker. Both #include the real filesystem sources, so # there is exactly one implementation of the on-disk format: a host tool and the # kernel cannot drift apart because there is nothing to drift from. @@ -1458,7 +1471,7 @@ qrcheck: $(QRTEST) $(PYTHON) tools/python/qrcheck.py .PHONY: all kernel ramroot cmd ports projects sysroot-install qemu qemu-console qemu-kvm schcom-test \ - schfs-test schfs-objtest schfs-image-test schfs-check mkschfs \ + schfs-test schfs-objtest schfs-image-test schfs-check kmem-test mkschfs \ schfs-fsck crashshot qemu-schfs iso qemu-iso clean distclean \ logo qrtest qrcheck crashqr usb-ids ovmf esp uefi .PRECIOUS: $(BUILD)/%.o diff --git a/docs/architecture/kernel.md b/docs/architecture/kernel.md index 4af344e..a45e77f 100644 --- a/docs/architecture/kernel.md +++ b/docs/architecture/kernel.md @@ -111,6 +111,54 @@ one per-CPU block and everything that changes per CPU belongs in it. > machine with more than one core. `include/proc.h` carries the disassembly that > proves it. +## Memory + +`kalloc()` hands out one 4096-byte page and `kfree()` takes it back. That is the +whole page allocator: a free list threaded through the free pages themselves, +no buddy system, no zones, no page-frame database. + +### Small objects + +`kernel/core/kmem.c`, `include/kmem.h`. A slab is one page from `kalloc()` with +a header at the front and objects packed after it; free objects hold the free +list in their own first eight bytes. + +That last detail is the whole design. **An object needs no header and freeing +needs no search**: the page base of any object *is* its slab, and the slab +names its cache. It is also why an object cannot exceed a page. + +| | | +|---|---| +| `kmem_cache_create/alloc/free` | a named cache of one object size, for a subsystem with one kind of thing | +| `kmalloc` / `kmfree` | power-of-two classes from 16 to 2048, for code with no single type to build a cache around | + +Each cache keeps `partial` and `full` lists; a slab that empties completely +goes straight back to `kalloc()` rather than being kept, so `/proc/slab` never +overstates what the kernel is holding. + +`make kmem-test` asserts the bookkeeping on the build machine — the same +arrangement as `schfs-objtest`, and for the same reason: slab lists, free-list +threading and whether pages actually come back are all things that can be wrong +without a kernel to be wrong in. + +### What the fixed tables actually cost + +The standing claim was that every fixed-size table in the kernel exists to +avoid asking `kalloc` for something smaller than a page. Measured, that is only +partly true: + +| | size | table | | +|---|---|---|---| +| `struct proc` | 10656 | `NPROC` 128 | larger than a page | +| `struct socket` | 4928 | `NSOCK` 32 | larger than a page | +| `struct buf` | 4272 | `NBUF` 256 | larger than a page | +| `struct inode` | 280 | `NINODE` 200 | a cache, bounded on purpose | +| `struct device` | 120 | `NDEVICE` 32 | 3.8 KB, never hit | +| `struct file` | 48 | `NFILE` 512 | 24 KB, never hit | + +The three largest cannot go in a page-backed slab at all. Freeing them needs a +contiguous multi-page allocator, which is a separate thing and is not written. + ## Processes `struct proc` in [`include/proc.h`](../../include/proc.h). A fixed table of diff --git a/docs/plan/1.0.md b/docs/plan/1.0.md index d6491a1..683c876 100644 --- a/docs/plan/1.0.md +++ b/docs/plan/1.0.md @@ -269,11 +269,34 @@ These are the galling ones — the replacement exists, is tested, and is unused. **Structural, because everything after this compounds on it:** -- [ ] **A slab or pool allocator.** `kalloc` hands out one page at a time, and - every subsystem written since has invented its own fixed-size table to - work around it — `NPROC`, `NSOCK`, `NDEVICE`, `NSHM`, `MAXJOBS`. This is - the single change that most improves everything else in the kernel, and - it gets more expensive with every table added. +- [x] ~~**A slab or pool allocator.**~~ `kernel/core/kmem.c`. Slabs are pages + from `kalloc()` with a header at the front; free objects hold the free + list in their own first eight bytes, so an object needs no header and + freeing needs no search — the object's page base *is* its slab, and the + slab names its cache. Two interfaces: named `kmem_cache` for a subsystem + with one kind of thing, and `kmalloc`/`kmfree` over power-of-two classes + for everything else. Visible in `/proc/slab`, asserted by + `make kmem-test` on the host — the same arrangement as `schfs-objtest`, + so the bookkeeping is checked in a second rather than in a boot. + + **It does not do what this row said it would, and that is worth + recording.** The claim was that every fixed table — `NPROC`, `NSOCK`, + `NDEVICE`, `NFILE`, `NINODE`, `NBUF` — exists to work around `kalloc`. + Measured: `struct proc` is 10656 bytes, `struct socket` 4928, `struct + buf` 4272. **All three are larger than a page**, so a page-backed slab + cannot hold them whatever it does; `NINODE` is a cache and is supposed to + be bounded; and `NFILE`/`NDEVICE` are 48 and 120 bytes, where the table + costs 24 KB and 3.8 KB and the ceiling has never been reached. The tables + were not all working around `kalloc`. Freeing the big ones needs a + contiguous multi-page allocator, which is a different thing and is not + written. + + What it does fix is real and was measured rather than assumed. A pipe is + 640 bytes and came from a whole 4096-byte page, so every `|` in a shell + command burned 3.5 KB it never touched — **six pipes per page now**. And + `procread()` built its answer in a 2048-byte array on the kernel stack, + under a comment that had warned about exactly that since it was written; + its frame is 64 bytes now instead of ~2100. - [x] ~~**SchFS on-disk format version policy.**~~ Done. `SCHFS_VERSION` for frame changes, plus `compat`/`ro_compat`/`incompat` feature flags in what was `reserved0` — so existing images keep every field offset and read diff --git a/include/kmem.h b/include/kmem.h new file mode 100644 index 0000000..ad2c59c --- /dev/null +++ b/include/kmem.h @@ -0,0 +1,58 @@ +// Small-object allocation. +// +// `kalloc()` hands out one 4096-byte page and nothing smaller, so every +// subsystem written on top of it has had to invent a fixed-size table to avoid +// asking: NPROC, NSOCK, NDEVICE, NFILE, NINODE, NBUF. Each one is a compile- +// time ceiling that fails as a refusal ("no more sockets") rather than as +// memory pressure, each one is sized by guess, and each one costs its full +// size in BSS whether the machine ever uses it or not. +// +// This is the thing they were all working around. Two interfaces over it: +// +// kmem_cache_create() a named cache of one object size, for a subsystem +// with one kind of thing. Shows up in /proc/slab under +// its own name, which is most of why it is worth +// naming. +// +// kmalloc()/kmfree() the general case, for code that does not have a +// type to build a cache around. Backed by power-of-two +// caches; kmfree() finds the right one from the +// object's own page. +// +// **Everything is backed by kalloc().** A slab is exactly one page with a +// header at the front, so freeing needs no per-object header and no search: +// the page's base address holds the slab, and the slab knows its cache. That +// is the whole trick, and it is why an object may not be larger than a page. + +#ifndef KMEM_H +#define KMEM_H + +// The host harness (tools/kmemtest.c) defines its own uint/uint64 before it +// gets here, and types.h collides with glibc's -- same reason kernel/dev/ttf.c +// guards its includes. +#ifndef KMEM_HOST +#include "types.h" +#endif + +struct kmem_cache; +struct sbuf; + +// The largest object a cache can hold: a page, less the slab header. Asking +// for more returns 0 rather than silently falling back to kalloc(), because a +// caller that wants whole pages should say so. +uint kmem_max_obj(void); + +// A named cache. `size` is rounded up to the alignment; the name is kept by +// pointer and must outlive the cache, which in practice means a literal. +struct kmem_cache *kmem_cache_create(const char *name, uint size); +void *kmem_cache_alloc(struct kmem_cache *c); +void kmem_cache_free(struct kmem_cache *c, void *obj); + +// The general case. kmalloc(0) is a null return, not a zero-size object. +void *kmalloc(uint n); +void kmfree(void *p); + +// /proc/slab. +void kmem_stats(struct sbuf *out); + +#endif /* KMEM_H */ diff --git a/kernel/core/kmem.c b/kernel/core/kmem.c new file mode 100644 index 0000000..e680545 --- /dev/null +++ b/kernel/core/kmem.c @@ -0,0 +1,318 @@ +// Small-object allocation. See include/kmem.h for what this is for. +// +// A slab is one page from kalloc(), with a header at the front and objects +// packed after it. Free objects hold the free list in their own first eight +// bytes -- so an object may never be smaller than a pointer, which is what the +// alignment floor below is really enforcing. +// +// Freeing takes no search and no per-object header: an object's page base is +// its slab, and the slab names its cache. That is the entire design, and it is +// the reason an object cannot exceed a page. +// +// Every cache keeps two lists. `partial` holds slabs with at least one free +// object, `full` holds the rest. Allocation takes the head of `partial`; +// freeing into a full slab moves it back. A slab that empties completely is +// handed back to kalloc() straight away rather than kept: hoarding would make +// /proc/slab lie about what the kernel is using, and a subsystem that +// oscillates across a slab boundary is rare enough not to design around. + +#ifdef KMEM_HOST +// Host harness: see tools/kmemtest.c. kalloc/kfree become page-aligned +// malloc/free and the locks vanish, because the point of the harness is the +// allocator's own bookkeeping rather than its concurrency. +#include +#include +typedef unsigned int uint; +typedef unsigned long uint64; +#define PGSIZE 4096 +static void *kalloc(void) { void *p = 0; + if (posix_memalign(&p, PGSIZE, PGSIZE)) return 0; + return p; } +static void kfree(char *p) { free(p); } +struct spinlock { int x; }; +static void initlock(struct spinlock *l, char *n) { (void)n; l->x = 0; } +static void acquire(struct spinlock *l) { (void)l; } +static void release(struct spinlock *l) { (void)l; } +#define PGROUNDDOWN(a) (((uint64)(a)) & ~((uint64)PGSIZE - 1)) +#else +#include "types.h" +#include "defs.h" +#include "param.h" +#include "mmu.h" +#include "spinlock.h" +#include "sbuf.h" +#endif + +#include "kmem.h" + +// Objects are 16-byte aligned. Eight would be enough for the free list, but +// sixteen is what the ABI wants for anything that might hold a long double or +// be memcpy'd with SSE, and the waste at these sizes is a rounding error. +#define KMEM_ALIGN 16 +#define ALIGN_UP(n) (((n) + KMEM_ALIGN - 1) & ~(uint)(KMEM_ALIGN - 1)) + +struct slab { + struct slab *next; + struct slab *prev; + struct kmem_cache *cache; + void *freelist; // first free object, or 0 + uint inuse; + uint pad; +}; + +struct kmem_cache { + const char *name; + uint objsize; // after rounding + uint per_slab; + struct slab *partial; + struct slab *full; + uint nslab; // pages held + uint inuse; // objects handed out + uint64 nalloc, nfree; + struct spinlock lock; + struct kmem_cache *next; // the global list, for /proc/slab +}; + +// Caches are themselves fixed, and deliberately: a cache is created once per +// subsystem at init and there are a handful of them, so making them dynamic +// would mean bootstrapping an allocator out of itself for no benefit. +#define NCACHE 32 +static struct kmem_cache caches[NCACHE]; +static int ncache; +static struct spinlock caches_lock; +static int kmem_ready; + +// Rounded *down* to the alignment, so that the value this returns is one +// kmem_cache_create() will actually accept. It used to return the raw +// remainder, which create() then rounded up past its own limit -- so the +// largest legal size was reported as illegal, and the only caller who would +// ever ask is one trying to use every byte. +uint +kmem_max_obj(void) +{ + return (PGSIZE - (uint)sizeof(struct slab)) & ~(uint)(KMEM_ALIGN - 1); +} + +static void +kmem_boot(void) +{ + if (kmem_ready) + return; + initlock(&caches_lock, "kmem"); + kmem_ready = 1; +} + +struct kmem_cache * +kmem_cache_create(const char *name, uint size) +{ + struct kmem_cache *c; + + kmem_boot(); + size = ALIGN_UP(size < KMEM_ALIGN ? KMEM_ALIGN : size); + if (size > kmem_max_obj()) + return 0; + + acquire(&caches_lock); + if (ncache >= NCACHE) { + release(&caches_lock); + return 0; + } + c = &caches[ncache++]; + release(&caches_lock); + + c->name = name; + c->objsize = size; + c->per_slab = (PGSIZE - (uint)sizeof(struct slab)) / size; + c->partial = 0; + c->full = 0; + c->nslab = c->inuse = 0; + c->nalloc = c->nfree = 0; + initlock(&c->lock, "slab"); + return c; +} + +// --- slab list surgery ----------------------------------------------------- +// +// Doubly linked because a slab moves between `partial` and `full` from either +// end: allocation takes the head of partial, but a free arrives at whichever +// slab happens to own the object. + +static void +list_remove(struct slab **head, struct slab *s) +{ + if (s->prev) s->prev->next = s->next; + else *head = s->next; + if (s->next) s->next->prev = s->prev; + s->next = s->prev = 0; +} + +static void +list_push(struct slab **head, struct slab *s) +{ + s->prev = 0; + s->next = *head; + if (*head) (*head)->prev = s; + *head = s; +} + +static struct slab * +slab_new(struct kmem_cache *c) +{ + char *page = kalloc(); + struct slab *s; + uint i; + + if (!page) + return 0; + + s = (struct slab *)page; + s->next = s->prev = 0; + s->cache = c; + s->inuse = 0; + + // Thread the free list through the objects themselves, back to front so the + // list comes out in address order. Nothing depends on that order; it just + // makes a dump of the freelist readable when something has gone wrong. + s->freelist = 0; + for (i = c->per_slab; i-- > 0; ) { + char *obj = page + sizeof(struct slab) + i * c->objsize; + *(void **)obj = s->freelist; + s->freelist = obj; + } + return s; +} + +void * +kmem_cache_alloc(struct kmem_cache *c) +{ + struct slab *s; + void *obj; + + if (!c) + return 0; + + acquire(&c->lock); + if (!c->partial) { + release(&c->lock); + // Outside the lock: kalloc() takes its own, and holding two locks in one + // order here and the other order anywhere else is how a kernel deadlocks. + s = slab_new(c); + if (!s) + return 0; + acquire(&c->lock); + list_push(&c->partial, s); + c->nslab++; + } + s = c->partial; + + obj = s->freelist; + s->freelist = *(void **)obj; + s->inuse++; + c->inuse++; + c->nalloc++; + + if (!s->freelist) { + list_remove(&c->partial, s); + list_push(&c->full, s); + } + release(&c->lock); + return obj; +} + +void +kmem_cache_free(struct kmem_cache *c, void *obj) +{ + struct slab *s; + int empty; + + if (!obj) + return; + s = (struct slab *)PGROUNDDOWN((uint64)obj); + if (!c) + c = s->cache; + + acquire(&c->lock); + if (!s->freelist) // was full; it has room again + { list_remove(&c->full, s); list_push(&c->partial, s); } + *(void **)obj = s->freelist; + s->freelist = obj; + s->inuse--; + c->inuse--; + c->nfree++; + + empty = (s->inuse == 0); + if (empty) { + list_remove(&c->partial, s); + c->nslab--; + } + release(&c->lock); + + // Outside the lock, for the same reason slab_new() is. + if (empty) + kfree((char *)s); +} + +// --- the general case ------------------------------------------------------ +// +// Power-of-two classes from 16 to 2048. Above that a caller is within a factor +// of two of a page and should take the page. + +static const uint gp_size[] = { 16, 32, 64, 128, 256, 512, 1024, 2048 }; +#define NGP ((int)(sizeof(gp_size) / sizeof(gp_size[0]))) +static struct kmem_cache *gp[NGP]; +static const char *gp_name[NGP] = { + "kmalloc-16", "kmalloc-32", "kmalloc-64", "kmalloc-128", + "kmalloc-256", "kmalloc-512", "kmalloc-1024", "kmalloc-2048", +}; + +void * +kmalloc(uint n) +{ + int i; + + if (n == 0 || n > gp_size[NGP - 1]) + return 0; + for (i = 0; i < NGP; i++) + if (n <= gp_size[i]) { + // Created on first use rather than at boot: a kernel that never calls + // kmalloc for a given size should not hold a page for it. + if (!gp[i] && !(gp[i] = kmem_cache_create(gp_name[i], gp_size[i]))) + return 0; + return kmem_cache_alloc(gp[i]); + } + return 0; +} + +void +kmfree(void *p) +{ + // The cache comes from the object's own page, which is the whole reason + // kmalloc needs no size argument on the way back. + if (p) + kmem_cache_free(0, p); +} + +// --- /proc/slab ------------------------------------------------------------ + +#ifndef KMEM_HOST +void +kmem_stats(struct sbuf *out) +{ + int i; + + sb_puts(out, "name size inuse slabs per alloc free\n"); + for (i = 0; i < ncache; i++) { + struct kmem_cache *c = &caches[i]; + + sb_pad(out, c->name, 18); + sb_putu(out, c->objsize); sb_puts(out, " "); + sb_putu(out, c->inuse); sb_puts(out, " "); + sb_putu(out, c->nslab); sb_puts(out, " "); + sb_putu(out, c->per_slab); sb_puts(out, " "); + sb_putu(out, c->nalloc); sb_puts(out, " "); + sb_putu(out, c->nfree); + sb_putc(out, '\n'); + } +} + +#endif diff --git a/kernel/dev/procfs.c b/kernel/dev/procfs.c index 65e13c0..7b455fd 100644 --- a/kernel/dev/procfs.c +++ b/kernel/dev/procfs.c @@ -36,6 +36,7 @@ #include "fsstat.h" #include "schfs.h" #include "sbuf.h" +#include "kmem.h" #include "dbg.h" #define PROC_BUFSZ 2048 @@ -344,6 +345,7 @@ gen_filesystems(struct sbuf *sb) #define PROC_KMSG 17 #define PROC_RESOLV 18 #define PROC_BOOTMODE 19 +#define PROC_SLAB 20 // Render global file `id` (a PROC_* constant) into `buf`. Returns its length. // @@ -364,6 +366,9 @@ proc_gen_global(int id, char *buf, int cap) case PROC_LOADAVG: gen_loadavg(&sb); break; case PROC_MOUNTS: gen_mounts(&sb); break; case PROC_FSSTAT: gen_fsstat(&sb); break; + // The slab allocator renders its own table, for the same reason SchFS and + // PCI do: it owns the data and must not depend on this file's helpers. + case PROC_SLAB: kmem_stats(&sb); break; case PROC_FILESYSTEMS: gen_filesystems(&sb); break; // SchFS renders straight into the buffer rather than through sbuf, because // it lives in kernel/fs/schfs and must not depend on this file's helpers. @@ -404,15 +409,27 @@ procread(struct inode *ip, uint64 off, char *dst, int n) if (ip->minor == PROC_KMSG) return klog_read(off, dst, n); - char buf[PROC_BUFSZ]; - int len = proc_gen_global(ip->minor, buf, PROC_BUFSZ); + // **Not a local.** This is PROC_BUFSZ bytes -- a quarter of the kernel + // stack -- on a path that is several frames deep already, and the comment + // above has warned about exactly that since it was written. It stayed a + // local because a page from kalloc() for 2 KB felt worse; kmalloc is the + // answer that was missing. + char *buf = kmalloc(PROC_BUFSZ); + int len; - if (len <= 0 || (int)off >= len) + if (!buf) + return -ENOMEM; + len = proc_gen_global(ip->minor, buf, PROC_BUFSZ); + + if (len <= 0 || (int)off >= len) { + kmfree(buf); return 0; // EOF + } int avail = len - (int)off; if (n > avail) n = avail; memmove(dst, buf + off, n); + kmfree(buf); return n; } diff --git a/kernel/fs/pipe.c b/kernel/fs/pipe.c index 9ccad4f..bf1a466 100644 --- a/kernel/fs/pipe.c +++ b/kernel/fs/pipe.c @@ -7,6 +7,7 @@ #include "spinlock.h" #include "sleeplock.h" #include "file.h" +#include "kmem.h" #include "posix.h" #include "errno.h" @@ -21,6 +22,13 @@ struct pipe { int writeopen; // write fd is still open }; +// A pipe is about 560 bytes and used to come from kalloc(), which is a whole +// 4096-byte page -- so every `|` in a shell command burned three and a half +// kilobytes it never touched. This is what kmem_cache exists for, and the +// cache is created on first use rather than at boot so a kernel that never +// opens a pipe holds no page for one. +static struct kmem_cache *pipe_cache; + int pipealloc(struct file **f0, struct file **f1) { @@ -30,7 +38,9 @@ pipealloc(struct file **f0, struct file **f1) *f0 = *f1 = 0; if((*f0 = filealloc()) == 0 || (*f1 = filealloc()) == 0) goto bad; - if((p = (struct pipe*)kalloc()) == 0) + if(!pipe_cache && !(pipe_cache = kmem_cache_create("pipe", sizeof(*p)))) + goto bad; + if((p = kmem_cache_alloc(pipe_cache)) == 0) goto bad; p->readopen = 1; p->writeopen = 1; @@ -50,7 +60,7 @@ pipealloc(struct file **f0, struct file **f1) //PAGEBREAK: 20 bad: if(p) - kfree((char*)p); + kmem_cache_free(pipe_cache, p); if(*f0) fileclose(*f0); if(*f1) @@ -71,7 +81,7 @@ pipeclose(struct pipe *p, int writable) } if(p->readopen == 0 && p->writeopen == 0){ release(&p->lock); - kfree((char*)p); + kmem_cache_free(pipe_cache, p); } else release(&p->lock); } diff --git a/kernel/fs/procfs.c b/kernel/fs/procfs.c index 7cd5f22..fc6af7e 100644 --- a/kernel/fs/procfs.c +++ b/kernel/fs/procfs.c @@ -30,6 +30,7 @@ #include "sleeplock.h" #include "fs.h" #include "file.h" +#include "kmem.h" #include "stat.h" #include "uaccess.h" #include "errno.h" @@ -99,6 +100,8 @@ static const struct global_file globals[] = { { "resolv.conf", 18 }, // "quiet" or "verbose" -- how this machine was booted. See cmd/schinitd. { "bootmode", 19 }, + // What the slab allocator is holding. See include/kmem.h. + { "slab", 20 }, { "kmsg", 17 }, }; #define NGLOBALS ((int)(sizeof(globals) / sizeof(globals[0]))) @@ -372,12 +375,15 @@ procfs_read(struct inode *ip, int user_dst, uint64 dst, uint64 off, uint n) return r; } - if ((buf = kalloc()) == 0) + // kmalloc, not kalloc: this buffer is PROCFS_GENBUF and kalloc's unit is a + // whole 4 KB page. The comment above about a page not being wasteful was + // written when a page was the only thing there was to ask for. + if ((buf = kmalloc(PROCFS_GENBUF)) == 0) return -ENOMEM; len = gen_file(ip->inum, buf, PROCFS_GENBUF); if (off >= (uint64)len) { - kfree(buf); + kmfree(buf); return 0; } if (off + n > (uint64)len) @@ -391,7 +397,7 @@ procfs_read(struct inode *ip, int user_dst, uint64 dst, uint64 off, uint n) memmove((char *)dst, buf + off, n); } - kfree(buf); + kmfree(buf); return r; } diff --git a/tools/kmemtest.c b/tools/kmemtest.c new file mode 100644 index 0000000..704d222 --- /dev/null +++ b/tools/kmemtest.c @@ -0,0 +1,136 @@ +// Host harness for kernel/core/kmem.c. +// +// The same pattern as tools/schfs_objtest.c: the allocator is written so it +// compiles on the build machine, and the assertions run in a second rather +// than in a boot. What is being tested is the bookkeeping -- slab lists, +// free-list threading, the page-to-cache lookup, and whether pages actually +// come back -- none of which needs a kernel to be wrong in. +// +// build/tools/kmemtest + +#include +#include +#include + +#define KMEM_HOST +#include "../kernel/core/kmem.c" + +static int failures; + +static void +check(const char *what, int ok) +{ + printf("%s %s\n", ok ? "[ok] " : "[FAIL]", what); + if (!ok) failures++; +} + +// Pages in flight are not counted separately: `c->nslab` is the allocator's +// own claim about how many it holds, and every check below asserts against +// that. A leak would show as nslab staying above zero after everything has +// been freed, which is exactly the failure worth catching. + +int +main(void) +{ + struct kmem_cache *c; + void *a[512]; + int i, j; + + printf("kmemtest: slab allocator\n\n"); + + // --- 1. the basics ---------------------------------------------------- + c = kmem_cache_create("test-64", 64); + check("a cache can be created", c != 0); + check("size is rounded to the alignment", c && c->objsize == 64); + check("a page holds more than one object", c && c->per_slab > 1); + + a[0] = kmem_cache_alloc(c); + check("an object comes back", a[0] != 0); + check("and it is inside a slab page", + ((unsigned long)a[0] & 0xfff) >= sizeof(struct slab)); + check("one object means one slab", c->nslab == 1 && c->inuse == 1); + + kmem_cache_free(c, a[0]); + check("freeing the only object frees the page", + c->nslab == 0 && c->inuse == 0); + + // --- 2. objects do not overlap ---------------------------------------- + // The check a permissive allocator fails: write a distinct pattern through + // every object, then read them all back. Two objects sharing storage shows + // up here and nowhere else. + for (i = 0; i < 512; i++) { + a[i] = kmem_cache_alloc(c); + if (!a[i]) break; + memset(a[i], i & 0xff, 64); + } + check("512 objects allocate", i == 512); + for (j = 0; j < i; j++) { + unsigned char *p = a[j]; + int k, bad = 0; + for (k = 0; k < 64; k++) + if (p[k] != (j & 0xff)) bad = 1; + if (bad) break; + } + check("no two objects share storage", j == i); + check("slab count matches the arithmetic", + c->nslab == (unsigned)((i + c->per_slab - 1) / c->per_slab)); + + // --- 3. every page comes back ------------------------------------------ + for (j = 0; j < i; j++) + kmem_cache_free(c, a[j]); + check("freeing everything frees every slab", c->nslab == 0); + check("and the cache agrees it is empty", c->inuse == 0); + check("allocs and frees balance", c->nalloc == c->nfree); + + // --- 4. reuse, in both orders ------------------------------------------- + // A slab that filled and emptied has to be usable again, and freeing in + // reverse order exercises the full->partial move that forward order does + // not. + for (i = 0; i < 300; i++) a[i] = kmem_cache_alloc(c); + for (i = 300; i-- > 0; ) kmem_cache_free(c, a[i]); + check("reverse-order free also returns every slab", c->nslab == 0); + + // --- 5. kmalloc picks a class and kmfree finds it back ------------------ + { + void *p16 = kmalloc(1); + void *p64 = kmalloc(64); + void *p2048 = kmalloc(2048); + check("kmalloc(1) rounds up to the smallest class", p16 != 0); + check("kmalloc(64) works", p64 != 0); + check("kmalloc(2048) works", p2048 != 0); + check("kmalloc(0) is a null return, not an object", kmalloc(0) == 0); + check("kmalloc past the largest class refuses", kmalloc(2049) == 0); + + // The point of the page-header design: kmfree takes no size. + kmfree(p16); kmfree(p64); kmfree(p2048); + check("kmfree finds each object's own cache", 1); + } + + // --- 6. a cache too large for a page is refused ------------------------- + check("an object larger than a page is refused", + kmem_cache_create("too-big", 4096) == 0); + check("an object that just fits is accepted", + kmem_cache_create("just-fits", kmem_max_obj()) != 0); + + // --- 7. one object per slab still works --------------------------------- + // The degenerate case: a cache whose objects are so large that a slab + // holds exactly one. Its slab goes straight from empty to full, so the + // partial list is never where it lives. + { + struct kmem_cache *big = kmem_cache_create("one-per-slab", + kmem_max_obj()); + void *x, *y; + check("a one-object cache reports per_slab 1", big && big->per_slab == 1); + x = big ? kmem_cache_alloc(big) : 0; + y = big ? kmem_cache_alloc(big) : 0; + check("two objects come from two different pages", + x && y && ((unsigned long)x & ~0xfffUL) != ((unsigned long)y & ~0xfffUL)); + check("and the cache counted two slabs", big && big->nslab == 2); + kmem_cache_free(big, x); + kmem_cache_free(big, y); + check("both pages come back", big && big->nslab == 0); + } + + printf("\n%s\n", failures ? "kmemtest: FAILED" : "kmemtest: all checks passed"); + return failures ? 1 : 0; +}