From 0000cc84e78d937ad989b55e2be48d137d629c50 Mon Sep 17 00:00:00 2001 From: ChuWeiChang Date: Thu, 14 May 2026 15:47:14 +0800 Subject: [PATCH 1/3] Downgrade MULTI sentinel in rev_host_clear host_to_vfd stayed MULTI even after all but one VFD aliasing a host FD had closed, breaking reverse lookups for the survivor. Add host_fd_refs to track share counts: on close, decrement and, when the count falls to 1, search for the surviving VFD and restore a direct mapping instead of leaving MULTI in place. Change-Id: I196608b0dfe31492786928f71ba4a1e4801fc532 --- mk/tests.mk | 1 + src/fd-table.c | 29 ++++++++++++++++--- src/fd-table.h | 4 +++ tests/unit/test-fd-table-refcount.c | 43 +++++++++++++++++++++++++++++ tests/unit/test-runner.c | 2 ++ 5 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 tests/unit/test-fd-table-refcount.c diff --git a/mk/tests.mk b/mk/tests.mk index 6617c53..a4cccf5 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -5,6 +5,7 @@ TEST_DIR = tests/unit TEST_SRCS = $(TEST_DIR)/test-runner.c \ $(TEST_DIR)/test-fd-table.c \ + $(TEST_DIR)/test-fd-table-refcount.c \ $(TEST_DIR)/test-path.c \ $(TEST_DIR)/test-mount.c \ $(TEST_DIR)/test-cli.c \ diff --git a/src/fd-table.c b/src/fd-table.c index 48b018a..7b40739 100644 --- a/src/fd-table.c +++ b/src/fd-table.c @@ -101,6 +101,8 @@ static inline void rev_host_set(struct kbox_fd_table *t, long host_fd, long vfd) if (host_fd < 0 || (uint64_t) host_fd >= KBOX_HOST_FD_REVERSE_MAX) return; + + t->host_fd_refs[host_fd]++; cur = t->host_to_vfd[host_fd]; if (cur == KBOX_HOST_VFD_NONE || cur == (int32_t) vfd) { t->host_to_vfd[host_fd] = (int32_t) vfd; @@ -116,7 +118,8 @@ static inline void rev_host_clear(struct kbox_fd_table *t, long host_fd, long vfd) { - if (host_fd < 0 || (uint64_t) host_fd >= KBOX_HOST_FD_REVERSE_MAX) + if (host_fd < 0 || (uint64_t) host_fd >= KBOX_HOST_FD_REVERSE_MAX || + t->host_fd_refs[host_fd] == 0) return; /* Only the single-holder case can be cleared authoritatively. If * the slot is MULTI, leave it: we cannot prove this is the last @@ -124,10 +127,26 @@ static inline void rev_host_clear(struct kbox_fd_table *t, * lookups correctly. If the slot is NONE or claims a different * vfd, we were not the indexed holder; nothing to do. */ - if (t->host_to_vfd[host_fd] == (int32_t) vfd) + + t->host_fd_refs[host_fd]--; + if (t->host_fd_refs[host_fd] == 0) { t->host_to_vfd[host_fd] = KBOX_HOST_VFD_NONE; -} + } else if (t->host_fd_refs[host_fd] == 1) { + /* 1. Try a normal search first */ + long cur_vfd = kbox_fd_table_find_by_host_fd(t, host_fd); + + /* 2. If the search tripped over the dying slot, hide it and search + * again */ + if (cur_vfd == vfd) { + struct kbox_fd_entry *e = fd_lookup(t, vfd); + e->host_fd = -1; + cur_vfd = kbox_fd_table_find_by_host_fd(t, host_fd); + e->host_fd = host_fd; + } + t->host_to_vfd[host_fd] = cur_vfd; + } +} static inline void lkl_ref_inc(struct kbox_fd_table *t, long lkl_fd) { if (lkl_fd >= 0 && (uint64_t) lkl_fd < KBOX_LKL_FD_REFMAX && @@ -154,8 +173,10 @@ void kbox_fd_table_init(struct kbox_fd_table *t) clear_fd_entry(&t->low_fds[i]); for (i = 0; i < KBOX_MID_FD_MAX; i++) clear_fd_entry(&t->mid_fds[i]); - for (i = 0; i < KBOX_HOST_FD_REVERSE_MAX; i++) + for (i = 0; i < KBOX_HOST_FD_REVERSE_MAX; i++) { t->host_to_vfd[i] = KBOX_HOST_VFD_NONE; + t->host_fd_refs[i] = 0; + } for (i = 0; i < KBOX_LKL_FD_REFMAX; i++) t->lkl_fd_refs[i] = 0; t->next_fd = KBOX_FD_BASE; diff --git a/src/fd-table.h b/src/fd-table.h index d02a7fb..8e0bb63 100644 --- a/src/fd-table.h +++ b/src/fd-table.h @@ -66,6 +66,10 @@ struct kbox_fd_table { * kbox_fd_table_find_by_host_fd() on the close() hot path. */ int32_t host_to_vfd[KBOX_HOST_FD_REVERSE_MAX]; + /* Tracks how many VFDs point to a specific Host FD. + * This allows us to know when to downgrade from MULTI. + */ + uint16_t host_fd_refs[KBOX_HOST_FD_REVERSE_MAX]; /* Refcount: how many virtual fds currently reference each * lkl_fd. Replaces the O(n) lkl_fd_has_other_ref scan and the * still_ref loop in forward_close. diff --git a/tests/unit/test-fd-table-refcount.c b/tests/unit/test-fd-table-refcount.c new file mode 100644 index 0000000..9ee3c0c --- /dev/null +++ b/tests/unit/test-fd-table-refcount.c @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: MIT */ +#include + +#include "fd-table.h" +#include "test-runner.h" +#define KBOX_HOST_VFD_NONE ((int32_t) -1) +#define KBOX_HOST_VFD_MULTI ((int32_t) -2) + +static void test_fd_table_multi_downgrade_regression(void) +{ + struct kbox_fd_table t; + long vfd1, vfd2; + long host_fd = 42; + + kbox_fd_table_init(&t); + + vfd1 = kbox_fd_table_insert(&t, 10, 0); + kbox_fd_table_set_host_fd(&t, vfd1, host_fd); + + ASSERT_EQ(t.host_to_vfd[host_fd], vfd1); + ASSERT_EQ(t.host_fd_refs[host_fd], 1); + + /* MULTI state*/ + vfd2 = kbox_fd_table_insert(&t, 20, 0); + kbox_fd_table_set_host_fd(&t, vfd2, host_fd); + + ASSERT_EQ(t.host_to_vfd[host_fd], KBOX_HOST_VFD_MULTI); + ASSERT_EQ(t.host_fd_refs[host_fd], 2); + + /* Downgrade back to Single*/ + kbox_fd_table_remove(&t, vfd2); + + ASSERT_EQ(t.host_to_vfd[host_fd], vfd1); + ASSERT_EQ(t.host_fd_refs[host_fd], 1); + + /* Should return the exact vfd */ + ASSERT_EQ(kbox_fd_table_find_by_host_fd(&t, host_fd), vfd1); +} + +void test_fd_table_refcount_init(void) +{ + TEST_REGISTER(test_fd_table_multi_downgrade_regression); +} diff --git a/tests/unit/test-runner.c b/tests/unit/test-runner.c index 52d3b33..3ed87ec 100644 --- a/tests/unit/test-runner.c +++ b/tests/unit/test-runner.c @@ -90,6 +90,7 @@ void test_pass(void) /* External init functions from each test file */ /* Portable test suites (all hosts) */ extern void test_fd_table_init(void); +extern void test_fd_table_refcount_init(void); extern void test_path_init(void); extern void test_mount_init(void); extern void test_cli_init(void); @@ -120,6 +121,7 @@ int main(int argc, char *argv[]) /* Portable suites */ test_fd_table_init(); + test_fd_table_refcount_init(); test_path_init(); test_mount_init(); test_cli_init(); From 0000b5bd635505d8357fa2d7b176b13ef3cd5868 Mon Sep 17 00:00:00 2001 From: ChuWeiChang Date: Thu, 28 May 2026 19:41:41 +0800 Subject: [PATCH 2/3] Add make check-perf Introduce a dedicated check-perf build target that compiles the unit test runner with -O2 -DKBOX_PERF_ONLY, suppressing TEST_REGISTER so only PERF_REGISTER benchmarks execute. The perf binary is kept separate from the unit test binary (test-perf vs test-runner) so running check-perf does not invalidate check-unit's build artifact. Change-Id: I55d7ff64b981dbbcf248b6ab675fd2c86147b165 --- .github/workflows/build-kbox.yml | 10 ++++++++++ .gitignore | 1 + mk/tests.mk | 18 ++++++++++++++++-- tests/unit/test-runner.h | 5 +++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-kbox.yml b/.github/workflows/build-kbox.yml index 6202d6f..8144000 100644 --- a/.github/workflows/build-kbox.yml +++ b/.github/workflows/build-kbox.yml @@ -91,6 +91,16 @@ jobs: - name: Run unit tests (ASAN/UBSAN) run: make check-unit + # ---- Perf tests: no sanitizers, only perf benchmarks ---- + perf-tests: + runs-on: ubuntu-24.04 + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Run perf tests + run: make check-perf + # ---- Build kbox + prepare rootfs ---- build-kbox: runs-on: ubuntu-24.04 diff --git a/.gitignore b/.gitignore index 9453641..242e956 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ __pycache__ *.ext4 /kbox tests/unit/test-runner +tests/unit/test-perf deps/ lkl-x86_64/ lkl-aarch64/ diff --git a/mk/tests.mk b/mk/tests.mk index a4cccf5..63e6992 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -56,6 +56,7 @@ TEST_SUPPORT_SRCS += $(SRC_DIR)/rewrite.c \ endif TEST_TARGET = tests/unit/test-runner +PERF_TARGET = tests/unit/test-perf # Guest test programs (compiled statically, run inside kbox) GUEST_DIR = tests/guest @@ -72,7 +73,7 @@ ROOTFS = alpine.ext4 # ---- Test targets ---- -check: check-unit check-integration check-stress +check: check-unit check-perf check-integration check-stress check-unit: $(TEST_TARGET) @echo " RUN check-unit" @@ -86,6 +87,19 @@ $(TEST_TARGET): $(TEST_SRCS) $(TEST_SUPPORT_SRCS) $(wildcard .config) @echo " LD $@" $(Q)$(CC) $(CFLAGS) -DKBOX_UNIT_TEST -o $@ $(TEST_SRCS) $(TEST_SUPPORT_SRCS) $(TEST_LDFLAGS) -lpthread +check-perf: $(PERF_TARGET) + @echo " RUN check-perf" + $(Q)./$(PERF_TARGET) + +# Perf-test binary: strip debug/sanitizer flags from CFLAGS, force -O2. +PERF_TEST_CFLAGS := $(filter-out -O0 -O1 -O3 -g -g3 -fsanitize% -fno-omit-frame-pointer,$(CFLAGS)) \ + -Wno-unused-function -O2 -DKBOX_UNIT_TEST -DKBOX_PERF_TESTS -DKBOX_PERF_ONLY +PERF_TEST_LDFLAGS := $(filter-out -L$(LKL_DIR) -L$(LKL_DIR)/lib -fsanitize%,$(LDFLAGS)) + +$(PERF_TARGET): $(TEST_SRCS) $(TEST_SUPPORT_SRCS) $(wildcard .config) + @echo " LD $@" + $(Q)$(CC) $(PERF_TEST_CFLAGS) -o $@ $(TEST_SRCS) $(TEST_SUPPORT_SRCS) $(PERF_TEST_LDFLAGS) -lpthread + check-integration: $(TARGET) guest-bins stress-bins $(ROOTFS) @echo " RUN check-integration" $(Q)./scripts/run-tests.sh ./$(TARGET) $(ROOTFS) @@ -164,4 +178,4 @@ check-commitlog: @echo " RUN check-commitlog" $(Q)scripts/check-commitlog.sh -.PHONY: check check-unit check-integration check-stress check-commitlog guest-bins stress-bins rootfs check-syntax +.PHONY: check check-unit check-perf check-integration check-stress check-commitlog guest-bins stress-bins rootfs check-syntax diff --git a/tests/unit/test-runner.h b/tests/unit/test-runner.h index f9c84a9..a9c6b51 100644 --- a/tests/unit/test-runner.h +++ b/tests/unit/test-runner.h @@ -77,6 +77,11 @@ void test_pass(void); } while (0) #endif +#ifdef KBOX_PERF_ONLY +#define TEST_REGISTER(fn) ((void) 0) +#define PERF_REGISTER(fn) test_register(#fn, fn) +#else #define TEST_REGISTER(fn) test_register(#fn, fn) +#endif #endif /* TEST_RUNNER_H */ From 00005425cdc606f6d58f920c68ddc872d230fdd0 Mon Sep 17 00:00:00 2001 From: ChuWeiChang Date: Thu, 28 May 2026 20:51:32 +0800 Subject: [PATCH 3/3] Add fd-table O(1) reverse-lookup perf test Benchmark kbox_fd_table_find_by_host_fd across table sizes of 64 to 16384 entries. Assert that per-lookup latency stays within 2x of the baseline at N=64, verifying that the host_to_vfd reverse map added in the O(1) refactor holds its constant-time characteristic. Change-Id: Ibcb7188e2c444f4636f669e73aeacd3980f04b84 --- Makefile | 2 +- tests/unit/test-fd-table-refcount.c | 80 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4b84747..3e782d4 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ KCONFIG_CONF := configs/Kconfig # Targets that don't require .config CONFIG_TARGETS := config defconfig oldconfig savedefconfig clean distclean indent \ - check-unit check-syntax check-commitlog fetch-lkl build-lkl \ + check-unit check-perf check-syntax check-commitlog fetch-lkl build-lkl \ fetch-minislirp install-hooks guest-bins stress-bins rootfs CONFIG_GENERATORS := config defconfig oldconfig diff --git a/tests/unit/test-fd-table-refcount.c b/tests/unit/test-fd-table-refcount.c index 9ee3c0c..90a9769 100644 --- a/tests/unit/test-fd-table-refcount.c +++ b/tests/unit/test-fd-table-refcount.c @@ -37,7 +37,87 @@ static void test_fd_table_multi_downgrade_regression(void) ASSERT_EQ(kbox_fd_table_find_by_host_fd(&t, host_fd), vfd1); } +#ifdef KBOX_PERF_TESTS +#include +#include +#define PERF_ITERATIONS 1000000 +static double time_diff_ns(struct timespec *start, struct timespec *end) +{ + return ((double) (end->tv_sec - start->tv_sec) * 1e9) + + (double) (end->tv_nsec - start->tv_nsec); +} + +static void test_fd_table_o1_characteristics(void) +{ + struct kbox_fd_table t; + int ns_to_test[] = {64, 256, 1024, 4096, 16384}; + int num_sizes = sizeof(ns_to_test) / sizeof(ns_to_test[0]); + + double baseline_present_ns = 0; + double baseline_absent_ns = 0; + + printf("\n--- O(1) Characteristic Perf Test ---\n"); + + for (int i = 0; i < num_sizes; i++) { + int n = ns_to_test[i]; + kbox_fd_table_init(&t); + long target_present = 0; + long target_absent = 65535; + + for (long j = 0, host_fd = 0; j < n; j++) { + long vfd = j; + kbox_fd_table_insert_at(&t, vfd, j, 0); + + /* linear congruential generator to prevent caching */ + host_fd = (16645 * host_fd + 10139) % 65536; + kbox_fd_table_set_host_fd(&t, vfd, host_fd); + if (j == n / 2) { + target_present = host_fd; + } + } + + struct timespec start, end; + double present_time_ns, absent_time_ns; + + long sum_present = 0; + long sum_absent = 0; + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int k = 0; k < PERF_ITERATIONS; k++) { + sum_present += kbox_fd_table_find_by_host_fd(&t, target_present); + } + clock_gettime(CLOCK_MONOTONIC, &end); + present_time_ns = time_diff_ns(&start, &end) / PERF_ITERATIONS; + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int k = 0; k < PERF_ITERATIONS; k++) { + sum_absent += kbox_fd_table_find_by_host_fd(&t, target_absent); + } + clock_gettime(CLOCK_MONOTONIC, &end); + absent_time_ns = time_diff_ns(&start, &end) / PERF_ITERATIONS; + + printf("N = %-5d | Present: %6.2f ns/op | Absent: %6.2f ns/op\n", n, + present_time_ns, absent_time_ns); + + if (i == 0) { + baseline_present_ns = present_time_ns; + baseline_absent_ns = absent_time_ns; + } else { + /* The per-lookup cost ratio across N must stay under ~2x */ + double present_ratio = present_time_ns / baseline_present_ns; + double absent_ratio = absent_time_ns / baseline_absent_ns; + + ASSERT_TRUE(present_ratio < 2); + ASSERT_TRUE(absent_ratio < 2); + } + } +} +#endif void test_fd_table_refcount_init(void) { TEST_REGISTER(test_fd_table_multi_downgrade_regression); + +#ifdef KBOX_PERF_TESTS + PERF_REGISTER(test_fd_table_o1_characteristics); +#endif }