[PATCH 0/2] vhost: set dirty log bits without pinning pages on x86

[PATCH 0/2] vhost: set dirty log bits without pinning pages on x86

[zhaoguohan]

From: Khan Zhao <zhaoguohan@kylinos.cn>

We use vhost-net with live migration in our product. While dirty
logging is enabled, every used ring update and every logged write goes
through set_bit_to_user(), which pins the log page, maps it, sets a
single bit and unpins it again. The TODO above that function has noted
for a long time that a direct user access would be better.

Patch 1 does that on x86: it tries a byte-sized cmpxchg on the user
address with page faults disabled, and falls back to the existing
pin/kmap path if the access faults or keeps losing to concurrent
updates of the same byte. Other architectures are unchanged, since
unsafe_try_cmpxchg_user() is only implemented on x86.

Patch 2 adds a userspace test in tools/virtio that drives used-ring
logging through vhost_test and checks the bitmap, including fault,
COW, shared-file and concurrent cases, plus a small benchmark mode.

On an x86_64 KVM guest (i7-12800HX host), the benchmark with logging
enabled went from 243.60 to 171.11 ns per descriptor, averaged over
five alternating runs. This is the per-descriptor cost in the vhost
worker; I have not measured the effect on total migration time.

Tool usage: the test, its documentation and the benchmark in patch 2
were largely generated with OpenAI Codex (gpt-6.1-sol) from a request
to exercise vhost used-ring dirty logging, and Codex helped run the
build and VM tests. The changelogs were edited with help from an AI
assistant. I have reviewed all of the code and take responsibility
for it.

Testing: x86_64 with SMAP under QEMU/KVM, running the test from
patch 2 against both the unmodified and patched vhost modules. i386
and arm64 are build-tested only; non-x86 has not been runtime tested.

Khan Zhao (2):
  vhost: set dirty log bits without pinning pages on x86
  tools/virtio: add a vhost dirty log test

 drivers/vhost/vhost.c           |  50 +++-
 tools/virtio/.gitignore         |   1 +
 tools/virtio/Makefile           |   7 +-
 tools/virtio/vhost_log_test.c   | 393 ++++++++++++++++++++++++++++++++
 tools/virtio/vhost_log_test.rst |  53 +++++
 5 files changed, 497 insertions(+), 7 deletions(-)
 create mode 100644 tools/virtio/vhost_log_test.c
 create mode 100644 tools/virtio/vhost_log_test.rst


base-commit: 9d66f26fefacebfeed15de31c1123451a17ff119
-- 
2.43.0

[PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

[zhaoguohan]

From: Khan Zhao <zhaoguohan@kylinos.cn>

set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
with kmap_atomic(), sets one bit and then unpins the page and marks it
dirty. This runs for every bit set in the dirty log, e.g. on each used
ring update while logging is enabled for live migration.

log_write() always passes a byte address and a bit number in [0, 7], so
the update can be done as a byte-sized cmpxchg on the user address. On
x86, try that first using unsafe_try_cmpxchg_user() inside a
user_access_begin()/user_access_end() section with page faults disabled.
If the access faults (page not present, read-only or needing COW), or
the cmpxchg loses to concurrent updates of the same byte four times in
a row, fall back to the existing pin/kmap/set_bit path, which can fault
the page in.

unsafe_try_cmpxchg_user() is only implemented on x86, so other
architectures keep using the existing path.

On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
workload with logging enabled went from 243.60 to 171.11 ns per
descriptor, averaged over five alternating runs. This is the
end-to-end cost per descriptor, not live migration performance.

Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
---
 drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e29f154706da..9c3045504709 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
 }
 EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
 
-/* TODO: This is really inefficient.  We need something like get_user()
- * (instruction directly accesses the data, with an exception table entry
- * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
- */
-static int set_bit_to_user(int nr, void __user *addr)
+static int set_bit_to_user_slow(int nr, void __user *addr)
 {
 	unsigned long log = (unsigned long)addr;
 	struct page *page;
@@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
 	return 0;
 }
 
+#ifdef CONFIG_X86
+static bool set_bit_to_user_fast(int nr, void __user *addr)
+{
+	u8 __user *p = addr;
+	u8 old;
+	int retries = 4;
+	bool success = false;
+
+	/*
+	 * log_write() passes a byte address and a bit number in [0, 7].
+	 * On x86, update just that byte to avoid crossing a page boundary.
+	 * Bound retries so a contended byte cannot hold up the worker.
+	 */
+	pagefault_disable();
+	if (!user_access_begin(p, sizeof(*p)))
+		goto out;
+	unsafe_get_user(old, p, fault);
+	do {
+		if (unsafe_try_cmpxchg_user(p, &old, old | BIT(nr), fault)) {
+			success = true;
+			break;
+		}
+	} while (--retries);
+fault:
+	user_access_end();
+out:
+	pagefault_enable();
+	return success;
+}
+#else
+static bool set_bit_to_user_fast(int nr, void __user *addr)
+{
+	return false;
+}
+#endif /* CONFIG_X86 */
+
+static int set_bit_to_user(int nr, void __user *addr)
+{
+	if (set_bit_to_user_fast(nr, addr))
+		return 0;
+	/* Resolve faults, or use the native bit layout on other architectures. */
+	return set_bit_to_user_slow(nr, addr);
+}
+
 static int log_write(void __user *log_base,
 		     u64 write_address, u64 write_length)
 {
-- 
2.43.0

[PATCH 2/2] tools/virtio: add a vhost dirty log test

[zhaoguohan]

From: Khan Zhao <zhaoguohan@kylinos.cn>

Add vhost_log_test, which exercises used-ring dirty logging through the
existing vhost_test module. It sets a log base with VHOST_SET_LOG_BASE,
enables VHOST_VRING_F_LOG on the ring, submits descriptors and checks
the resulting bitmap from userspace. No new kernel interface is needed.

The test covers:

 - every bit in a byte, repeated sets, and neighbouring bytes being left
   untouched, including the last byte of a page followed by a PROT_NONE
   guard page
 - a log page that has not been faulted in yet, read-only, PROT_NONE
   and unmapped log pages, and logging after access is restored
 - private (COW) and shared file mappings, checking that the bit reaches
   the file after msync/fsync
 - eight vrings setting different bits of the same byte concurrently,
   with and without a userspace thread atomically fetching and clearing
   that byte
 - two shared mappings of one file page, one of them zapped with
   MADV_DONTNEED each round, so both resident and faulting pages are
   hit

log_used() ignores errors from log_write(), so the fault cases can only
check that nothing crashes, that nothing is written to an inaccessible
log, and that logging works again afterwards.

A --bench mode reports ns per descriptor with logging disabled and
enabled. vhost_log_test.rst describes how to build and run the test.

Assisted-by: Codex:gpt-6.1-sol
Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
---
 tools/virtio/.gitignore         |   1 +
 tools/virtio/Makefile           |   7 +-
 tools/virtio/vhost_log_test.c   | 393 ++++++++++++++++++++++++++++++++
 tools/virtio/vhost_log_test.rst |  53 +++++
 4 files changed, 452 insertions(+), 2 deletions(-)
 create mode 100644 tools/virtio/vhost_log_test.c
 create mode 100644 tools/virtio/vhost_log_test.rst

diff --git a/tools/virtio/.gitignore b/tools/virtio/.gitignore
index 7e47b281c442..9f3ebfa2c3b6 100644
--- a/tools/virtio/.gitignore
+++ b/tools/virtio/.gitignore
@@ -2,5 +2,6 @@
 *.d
 virtio_test
 vhost_net_test
+vhost_log_test
 vringh_test
 virtio-trace/trace-agent
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index 2cac5fd4b979..a4af5140a1cd 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -1,9 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0
 all: test mod
-test: virtio_test vringh_test vhost_net_test
+test: virtio_test vringh_test vhost_net_test vhost_log_test
 virtio_test: virtio_ring.o virtio_test.o
 vringh_test: vringh_test.o vringh.o virtio_ring.o
 vhost_net_test: virtio_ring.o vhost_net_test.o
+vhost_log_test: vhost_log_test.o
+# This test uses only the userspace API, without the virtio kernel shims.
+vhost_log_test vhost_log_test.o: CFLAGS = -g -O2 -Wall -Wextra -Werror -pthread -I../../usr/include
 
 try-run = $(shell set -e;		\
 	if ($(1)) >/dev/null 2>&1;	\
@@ -52,7 +55,7 @@ oot-clean: OOT_BUILD+=clean
 
 .PHONY: all test mod clean vhost oot oot-clean oot-build
 clean:
-	${RM} *.o vringh_test virtio_test vhost_net_test vhost_test/*.o \
+	${RM} *.o vringh_test virtio_test vhost_net_test vhost_log_test vhost_test/*.o \
               vhost_test/.*.cmd vhost_test/Module.symvers \
               vhost_test/modules.order *.d
 -include *.d
diff --git a/tools/virtio/vhost_log_test.c b/tools/virtio/vhost_log_test.c
new file mode 100644
index 000000000000..96e58bbcdfdc
--- /dev/null
+++ b/tools/virtio/vhost_log_test.c
@@ -0,0 +1,393 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Exercise vhost used-ring dirty logging through /dev/vhost-test. */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <linux/vhost.h>
+#include <linux/virtio_ring.h>
+#include <poll.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/eventfd.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <time.h>
+#include <unistd.h>
+#include "../../drivers/vhost/test.h"
+
+#define PAGE_BYTES 4096
+#define RING_NUM 128
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+#define CHECK(c) do { \
+	if (!(c)) { \
+		fprintf(stderr, "FAIL line %d: %s (errno=%d: %s)\n", \
+			__LINE__, #c, errno, strerror(errno)); \
+		exit(EXIT_FAILURE); \
+	} \
+} while (0)
+
+struct queue {
+	int fd;
+	int kick;
+	int call;
+	void *ring;
+	void *data;
+	struct vring_desc *desc;
+	struct vring_avail *avail;
+	struct vring_used *used;
+	uint16_t index;
+};
+
+static void *map_pages(size_t size)
+{
+	void *p = mmap(NULL, size, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+	CHECK(p != MAP_FAILED);
+	return p;
+}
+
+static void log_base(struct queue *q, void *base)
+{
+	uint64_t addr = (uintptr_t)base;
+
+	CHECK(!ioctl(q->fd, VHOST_SET_LOG_BASE, &addr));
+}
+
+static void queue_init(struct queue *q, void *bitmap, unsigned int bit,
+		       bool logging)
+{
+	struct vhost_vring_state state = { .index = 0, .num = RING_NUM };
+	struct vhost_vring_file file = { .index = 0 };
+	struct vhost_vring_addr addr = { .index = 0 };
+	struct vhost_memory *mem;
+	uint64_t features = 0;
+	unsigned char *scratch;
+	int i, run = 1;
+
+	memset(q, 0, sizeof(*q));
+	q->fd = open("/dev/vhost-test", O_RDWR);
+	CHECK(q->fd >= 0);
+	CHECK(!ioctl(q->fd, VHOST_SET_OWNER, NULL));
+	CHECK(!ioctl(q->fd, VHOST_SET_FEATURES, &features));
+	q->kick = eventfd(0, EFD_NONBLOCK);
+	q->call = eventfd(0, EFD_NONBLOCK);
+	CHECK(q->kick >= 0 && q->call >= 0);
+	q->ring = map_pages(3 * PAGE_BYTES);
+	q->data = map_pages(PAGE_BYTES);
+	q->desc = q->ring;
+	q->avail = q->ring + PAGE_BYTES;
+	q->used = q->ring + 2 * PAGE_BYTES;
+	mem = calloc(1, sizeof(*mem) + sizeof(mem->regions[0]));
+	CHECK(mem);
+	mem->nregions = 1;
+	mem->regions[0].guest_phys_addr = 0;
+	mem->regions[0].userspace_addr = (uintptr_t)q->data;
+	mem->regions[0].memory_size = PAGE_BYTES;
+	CHECK(!ioctl(q->fd, VHOST_SET_MEM_TABLE, mem));
+	free(mem);
+	for (i = 0; i < RING_NUM; i++) {
+		q->desc[i].addr = 0;
+		q->desc[i].len = 1;
+	}
+	CHECK(!ioctl(q->fd, VHOST_SET_VRING_NUM, &state));
+	state.num = 0;
+	CHECK(!ioctl(q->fd, VHOST_SET_VRING_BASE, &state));
+	addr.desc_user_addr = (uintptr_t)q->desc;
+	addr.avail_user_addr = (uintptr_t)q->avail;
+	addr.used_user_addr = (uintptr_t)q->used;
+	addr.flags = logging ? 1U << VHOST_VRING_F_LOG : 0;
+	addr.log_guest_addr = (uint64_t)bit * PAGE_BYTES;
+	CHECK(!ioctl(q->fd, VHOST_SET_VRING_ADDR, &addr));
+	file.fd = q->kick;
+	CHECK(!ioctl(q->fd, VHOST_SET_VRING_KICK, &file));
+	file.fd = q->call;
+	CHECK(!ioctl(q->fd, VHOST_SET_VRING_CALL, &file));
+	/* Starting a queue logs its flags; keep the tested mapping untouched. */
+	scratch = map_pages(PAGE_BYTES);
+	log_base(q, scratch);
+	CHECK(!ioctl(q->fd, VHOST_TEST_RUN, &run));
+	log_base(q, bitmap);
+	CHECK(!munmap(scratch, PAGE_BYTES));
+}
+
+static void queue_submit(struct queue *q, unsigned int count)
+{
+	uint64_t kick = 1;
+	unsigned int i;
+
+	CHECK(count <= RING_NUM);
+	for (i = 0; i < count; i++)
+		q->avail->ring[(q->index + i) % RING_NUM] = i;
+	q->index += count;
+	__atomic_store_n(&q->avail->idx, q->index, __ATOMIC_RELEASE);
+	CHECK(write(q->kick, &kick, sizeof(kick)) == sizeof(kick));
+}
+
+static void queue_wait(struct queue *q)
+{
+	struct pollfd pfd = { .fd = q->call, .events = POLLIN };
+	uint64_t events;
+
+	/* Completion notification follows the dirty-log stores. */
+	do {
+		CHECK(poll(&pfd, 1, 5000) == 1);
+		CHECK(read(q->call, &events, sizeof(events)) == sizeof(events));
+	} while (__atomic_load_n(&q->used->idx, __ATOMIC_ACQUIRE) != q->index);
+}
+
+static void queue_destroy(struct queue *q)
+{
+	/* Release flushes the worker before freeing any userspace mappings. */
+	CHECK(!close(q->fd));
+	CHECK(!close(q->kick));
+	CHECK(!close(q->call));
+	CHECK(!munmap(q->ring, 3 * PAGE_BYTES));
+	CHECK(!munmap(q->data, PAGE_BYTES));
+}
+
+static void exercise(void *bitmap, unsigned int bit, unsigned int count)
+{
+	struct queue q;
+
+	queue_init(&q, bitmap, bit, true);
+	queue_submit(&q, count);
+	queue_wait(&q);
+	queue_destroy(&q);
+}
+
+static void basic_test(void)
+{
+	unsigned char *p = map_pages(2 * PAGE_BYTES);
+	unsigned int offsets[] = { 0, 1, 7, 63, PAGE_BYTES - 1 };
+	unsigned int i, bit;
+
+	CHECK(!mprotect(p + PAGE_BYTES, PAGE_BYTES, PROT_NONE));
+	for (i = 0; i < ARRAY_SIZE(offsets); i++) {
+		for (bit = 0; bit < 8; bit++) {
+			memset(p, 0, PAGE_BYTES);
+			exercise(p + offsets[i], bit, RING_NUM);
+			CHECK(p[offsets[i]] == (1U << bit));
+			if (offsets[i])
+				CHECK(!p[offsets[i] - 1]);
+			if (offsets[i] + 1 < PAGE_BYTES)
+				CHECK(!p[offsets[i] + 1]);
+			p[offsets[i]] = 0xa5;
+			exercise(p + offsets[i], bit, 1);
+			CHECK(p[offsets[i]] == (0xa5 | (1U << bit)));
+		}
+	}
+	CHECK(!munmap(p, 2 * PAGE_BYTES));
+	puts("PASS bits 0..7, repeated sets, unaligned and guarded page end");
+}
+
+static void fault_test(void)
+{
+	unsigned char *p = map_pages(PAGE_BYTES);
+	unsigned char resident;
+	struct queue q;
+
+	queue_init(&q, p, 3, true);
+	CHECK(!mincore(p, PAGE_BYTES, &resident));
+	CHECK(!(resident & 1));
+	queue_submit(&q, 1);
+	queue_wait(&q);
+	queue_destroy(&q);
+	CHECK(*p == 8);
+	*p = 0;
+	CHECK(!mprotect(p, PAGE_BYTES, PROT_READ));
+	exercise(p, 2, 1);
+	CHECK(!*p);
+	CHECK(!mprotect(p, PAGE_BYTES, PROT_NONE));
+	exercise(p, 2, 1);
+	CHECK(!mprotect(p, PAGE_BYTES, PROT_READ | PROT_WRITE));
+	exercise(p, 4, 1);
+	CHECK(*p == 16);
+	queue_init(&q, p, 2, true);
+	CHECK(!munmap(p, PAGE_BYTES));
+	queue_submit(&q, 1);
+	queue_wait(&q);
+	queue_destroy(&q);
+	/* Used-ring logging ignores errors; check survival and no bad write. */
+	puts("PASS first touch, readonly, PROT_NONE, unmapped, recovery");
+}
+
+static void file_test(const char *path)
+{
+	unsigned char *p;
+	unsigned char byte;
+	int fd = open(path, O_CREAT | O_TRUNC | O_RDWR, 0600);
+
+	CHECK(fd >= 0);
+	CHECK(!ftruncate(fd, PAGE_BYTES));
+	p = mmap(NULL, PAGE_BYTES, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	CHECK(p != MAP_FAILED);
+	CHECK(!*p);
+	exercise(p, 5, 1);
+	CHECK(*p == 32);
+	CHECK(pread(fd, &byte, 1, 0) == 1);
+	CHECK(!byte);
+	CHECK(!munmap(p, PAGE_BYTES));
+	p = mmap(NULL, PAGE_BYTES, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	CHECK(p != MAP_FAILED);
+	*p = 0;
+	CHECK(!msync(p, PAGE_BYTES, MS_SYNC));
+	exercise(p, 6, 1);
+	CHECK(*p == 64);
+	CHECK(!msync(p, PAGE_BYTES, MS_SYNC));
+	CHECK(!munmap(p, PAGE_BYTES));
+	CHECK(!fsync(fd));
+	CHECK(!posix_fadvise(fd, 0, PAGE_BYTES, POSIX_FADV_DONTNEED));
+	CHECK(pread(fd, &byte, 1, 0) == 1);
+	CHECK(byte == 64);
+	CHECK(!close(fd));
+	CHECK(!unlink(path));
+	puts("PASS private file COW and shared file sync/re-read");
+}
+
+struct collector {
+	unsigned char *bitmap;
+	bool stop;
+	unsigned char seen;
+};
+
+static void *collect(void *arg)
+{
+	struct collector *c = arg;
+
+	while (!__atomic_load_n(&c->stop, __ATOMIC_ACQUIRE))
+		c->seen |= __atomic_exchange_n(c->bitmap, 0, __ATOMIC_SEQ_CST);
+	return NULL;
+}
+
+static void concurrent_test(void)
+{
+	unsigned char *p = map_pages(PAGE_BYTES);
+	struct queue q[8];
+	struct collector c = { .bitmap = p };
+	pthread_t thread;
+	unsigned int bit, round;
+
+	for (bit = 0; bit < 8; bit++)
+		queue_init(&q[bit], p, bit, true);
+	for (round = 0; round < 1000; round++) {
+		__atomic_store_n(p, 0, __ATOMIC_SEQ_CST);
+		for (bit = 0; bit < 8; bit++)
+			queue_submit(&q[bit], 1);
+		for (bit = 0; bit < 8; bit++)
+			queue_wait(&q[bit]);
+		CHECK(__atomic_load_n(p, __ATOMIC_SEQ_CST) == 255);
+	}
+	for (round = 0; round < 1000; round++) {
+		__atomic_store_n(p, 0, __ATOMIC_SEQ_CST);
+		c.stop = false;
+		c.seen = 0;
+		CHECK(!pthread_create(&thread, NULL, collect, &c));
+		for (bit = 0; bit < 8; bit++)
+			queue_submit(&q[bit], RING_NUM);
+		for (bit = 0; bit < 8; bit++)
+			queue_wait(&q[bit]);
+		__atomic_store_n(&c.stop, true, __ATOMIC_RELEASE);
+		CHECK(!pthread_join(thread, NULL));
+		c.seen |= __atomic_exchange_n(p, 0, __ATOMIC_SEQ_CST);
+		CHECK(c.seen == 255);
+	}
+	for (bit = 0; bit < 8; bit++)
+		queue_destroy(&q[bit]);
+	CHECK(!munmap(p, PAGE_BYTES));
+	puts("PASS eight workers sharing a byte, concurrent atomic collection");
+}
+
+static void alias_test(const char *path)
+{
+	unsigned char *p, *alias;
+	struct queue q[8];
+	unsigned int bit, round;
+	int fd = open(path, O_CREAT | O_TRUNC | O_RDWR, 0600);
+
+	CHECK(fd >= 0);
+	CHECK(!ftruncate(fd, PAGE_BYTES));
+	p = mmap(NULL, PAGE_BYTES, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	alias = mmap(NULL, PAGE_BYTES, PROT_READ | PROT_WRITE, MAP_SHARED,
+		     fd, 0);
+	CHECK(p != MAP_FAILED && alias != MAP_FAILED);
+	for (bit = 0; bit < 8; bit++)
+		queue_init(&q[bit], bit & 1 ? alias : p, bit, true);
+	for (round = 0; round < 1000; round++) {
+		__atomic_store_n(p, 0, __ATOMIC_SEQ_CST);
+		/* Keep one alias writable, and force faults through the other. */
+		CHECK(!madvise(alias, PAGE_BYTES, MADV_DONTNEED));
+		for (bit = 0; bit < 8; bit++)
+			queue_submit(&q[bit], 1);
+		for (bit = 0; bit < 8; bit++)
+			queue_wait(&q[bit]);
+		CHECK(__atomic_load_n(p, __ATOMIC_SEQ_CST) == 255);
+	}
+	for (bit = 0; bit < 8; bit++)
+		queue_destroy(&q[bit]);
+	CHECK(!munmap(p, PAGE_BYTES));
+	CHECK(!munmap(alias, PAGE_BYTES));
+	CHECK(!close(fd));
+	CHECK(!unlink(path));
+	puts("PASS shared-file aliases with repeated PTE faults");
+}
+
+static uint64_t now_ns(void)
+{
+	struct timespec ts;
+
+	CHECK(!clock_gettime(CLOCK_MONOTONIC, &ts));
+	return (uint64_t)ts.tv_sec * 1000000000 + ts.tv_nsec;
+}
+
+static void benchmark(bool logging, unsigned int batches)
+{
+	unsigned char *p = map_pages(PAGE_BYTES);
+	struct queue q;
+	uint64_t start, elapsed;
+	unsigned int i;
+
+	*p = 0;
+	queue_init(&q, p, 0, logging);
+	for (i = 0; i < 100; i++) {
+		queue_submit(&q, RING_NUM);
+		queue_wait(&q);
+	}
+	start = now_ns();
+	for (i = 0; i < batches; i++) {
+		queue_submit(&q, RING_NUM);
+		queue_wait(&q);
+	}
+	elapsed = now_ns() - start;
+	queue_destroy(&q);
+	CHECK(*p == (logging ? 1 : 0));
+	printf("BENCH logging=%u descriptors=%u elapsed_ns=%" PRIu64
+	       " ns_per_desc=%.2f\n", logging, batches * RING_NUM,
+	       elapsed, (double)elapsed / (batches * RING_NUM));
+	CHECK(!munmap(p, PAGE_BYTES));
+}
+
+int main(int argc, char **argv)
+{
+	const char *path = argc > 1 ? argv[1] : "/tmp/vhost-log-test";
+
+	CHECK(sysconf(_SC_PAGESIZE) == PAGE_BYTES);
+	if (argc == 2 && !strcmp(argv[1], "--bench")) {
+		benchmark(false, 10000);
+		benchmark(true, 10000);
+		return 0;
+	}
+	basic_test();
+	fault_test();
+	file_test(path);
+	concurrent_test();
+	alias_test(path);
+	puts("PASS vhost dirty-log regression suite");
+	return 0;
+}
diff --git a/tools/virtio/vhost_log_test.rst b/tools/virtio/vhost_log_test.rst
new file mode 100644
index 000000000000..7652506e7400
--- /dev/null
+++ b/tools/virtio/vhost_log_test.rst
@@ -0,0 +1,53 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Vhost dirty-log regression test
+==============================
+
+``vhost_log_test`` exercises used-ring logging through the existing
+``/dev/vhost-test`` interface. It requires a test kernel with vhost support
+and the ``vhost_test`` module built from ``tools/virtio/vhost_test`` for that
+same kernel. Run it in a disposable VM, with permission to open the device.
+
+Build the userspace executable from the kernel source root::
+
+  make -C tools/virtio vhost_log_test
+
+For a static initramfs executable, use ``LDFLAGS="-static -pthread"`` on a
+clean build. Load ``vhost``, its dependencies, and ``vhost_test`` in the
+guest, then run::
+
+  ./vhost_log_test /mnt/vhost-log-scratch
+
+The path is a disposable scratch file: the test truncates and removes it.
+Use a disk-backed filesystem such as ext4 to exercise file writeback.
+Omitting the path uses ``/tmp/vhost-log-test``. The test currently requires
+4 KiB pages and the native little-endian legacy vring layout; runtime
+coverage is intended for x86.
+
+The suite covers every bit within a byte, repeated sets, neighboring-byte
+preservation, unaligned byte addresses, a guarded page end, first touch,
+inaccessible mappings and recovery, private COW, shared-file sync/reread,
+eight workers sharing a byte, and concurrent userspace atomic collection.
+The collector checks each of 1000 independent rounds. Another 1000 rounds
+use two aliases of a shared file page and discard one alias's PTEs before
+each round, exercising resident and faulting accesses to the same bitmap.
+
+Used-ring logging currently discards ``log_used()`` errors. Consequently,
+the inaccessible-mapping cases check survival, absence of a successful
+write and recovery; they do not verify userspace-visible ``-EFAULT``.
+The collector verifies that all eight bits were observed in each round,
+not that every repeated dirty event was counted. Alias tests exercise
+fault fallback but do not establish a deterministic instruction ordering.
+These tests do not replace MM race analysis or live migration testing.
+
+Run the benchmark separately::
+
+  taskset -c 0 ./vhost_log_test --bench
+
+Each logging-disabled/enabled measurement warms up with 100 batches,
+then times 10000 batches of 128 descriptors. The result includes queue
+submission, worker execution and completion notification. It measures
+nanoseconds per descriptor, not isolated ``set_bit_to_user()`` latency,
+network throughput or migration time. Keep the kernel configuration and
+VM setup identical, alternate baseline/optimized runs, and report multiple
+samples. Affinity applies to the guest CPU; host CPU placement is separate.
-- 
2.43.0

Re: [PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

[Michael S. Tsirkin]

On Thu, Nov 12, 2026 at 04:56:30PM +0800, zhaoguohan@kylinos.cn wrote:
> From: Khan Zhao <zhaoguohan@kylinos.cn>
> 
> set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
> with kmap_atomic(), sets one bit and then unpins the page and marks it
> dirty. This runs for every bit set in the dirty log, e.g. on each used
> ring update while logging is enabled for live migration.
> 
> log_write() always passes a byte address and a bit number in [0, 7], so
> the update can be done as a byte-sized cmpxchg on the user address. On
> x86, try that first using unsafe_try_cmpxchg_user() inside a
> user_access_begin()/user_access_end() section with page faults disabled.
> If the access faults (page not present, read-only or needing COW), or
> the cmpxchg loses to concurrent updates of the same byte four times in
> a row, fall back to the existing pin/kmap/set_bit path, which can fault
> the page in.
> 
> unsafe_try_cmpxchg_user() is only implemented on x86, so other
> architectures keep using the existing path.
> 
> On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
> workload with logging enabled went from 243.60 to 171.11 ns per
> descriptor, averaged over five alternating runs. This is the
> end-to-end cost per descriptor, not live migration performance.
> 
> Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
> ---
>  drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e29f154706da..9c3045504709 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
>  
> -/* TODO: This is really inefficient.  We need something like get_user()
> - * (instruction directly accesses the data, with an exception table entry
> - * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
> - */
> -static int set_bit_to_user(int nr, void __user *addr)
> +static int set_bit_to_user_slow(int nr, void __user *addr)
>  {
>  	unsigned long log = (unsigned long)addr;
>  	struct page *page;
> @@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_X86
> +static bool set_bit_to_user_fast(int nr, void __user *addr)
> +{
> +	u8 __user *p = addr;
> +	u8 old;
> +	int retries = 4;
> +	bool success = false;
> +
> +	/*
> +	 * log_write() passes a byte address and a bit number in [0, 7].
> +	 * On x86, update just that byte to avoid crossing a page boundary.
> +	 * Bound retries so a contended byte cannot hold up the worker.
> +	 */
> +	pagefault_disable();
> +	if (!user_access_begin(p, sizeof(*p)))
> +		goto out;
> +	unsafe_get_user(old, p, fault);
> +	do {
> +		if (unsafe_try_cmpxchg_user(p, &old, old | BIT(nr), fault)) {
> +			success = true;
> +			break;
> +		}
> +	} while (--retries);
> +fault:
> +	user_access_end();
> +out:
> +	pagefault_enable();
> +	return success;
> +}

Yes we wanted this for the longest time!
But can we use the nice scoped_user_rw_access API?
Something like:


        pagefault_disable();
        scoped_user_rw_access(p, efault) {
                unsafe_get_user(old, p, efault);
                do {
                        if (unsafe_try_cmpxchg_user(p, &old, old | BIT(nr), efault)) {
                                success = true;
                                break;
                        }
                } while (--retries);
        }
        pagefault_enable();
        return success;
efault:
        pagefault_enable();
        return false;


maybe?



> +#else
> +static bool set_bit_to_user_fast(int nr, void __user *addr)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_X86 */
> +
> +static int set_bit_to_user(int nr, void __user *addr)
> +{
> +	if (set_bit_to_user_fast(nr, addr))
> +		return 0;
> +	/* Resolve faults, or use the native bit layout on other architectures. */
> +	return set_bit_to_user_slow(nr, addr);
> +}
> +
>  static int log_write(void __user *log_base,
>  		     u64 write_address, u64 write_length)
>  {
> -- 
> 2.43.0

Re: [PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

[Michael S. Tsirkin]

On Thu, Nov 12, 2026 at 04:56:30PM +0800, zhaoguohan@kylinos.cn wrote:
> From: Khan Zhao <zhaoguohan@kylinos.cn>
> 
> set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
> with kmap_atomic(), sets one bit and then unpins the page and marks it
> dirty. This runs for every bit set in the dirty log, e.g. on each used
> ring update while logging is enabled for live migration.
> 
> log_write() always passes a byte address and a bit number in [0, 7], so
> the update can be done as a byte-sized cmpxchg on the user address. On
> x86, try that first using unsafe_try_cmpxchg_user() inside a
> user_access_begin()/user_access_end() section with page faults disabled.
> If the access faults (page not present, read-only or needing COW), or
> the cmpxchg loses to concurrent updates of the same byte four times in
> a row, fall back to the existing pin/kmap/set_bit path, which can fault
> the page in.
> 
> unsafe_try_cmpxchg_user() is only implemented on x86, so other
> architectures keep using the existing path.
> 
> On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
> workload with logging enabled went from 243.60 to 171.11 ns per
> descriptor, averaged over five alternating runs. This is the
> end-to-end cost per descriptor, not live migration performance.
> 
> Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
> ---
>  drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e29f154706da..9c3045504709 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
>  
> -/* TODO: This is really inefficient.  We need something like get_user()
> - * (instruction directly accesses the data, with an exception table entry
> - * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
> - */
> -static int set_bit_to_user(int nr, void __user *addr)
> +static int set_bit_to_user_slow(int nr, void __user *addr)
>  {
>  	unsigned long log = (unsigned long)addr;
>  	struct page *page;
> @@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_X86
> +static bool set_bit_to_user_fast(int nr, void __user *addr)
> +{
> +	u8 __user *p = addr;
> +	u8 old;
> +	int retries = 4;
> +	bool success = false;
> +
> +	/*
> +	 * log_write() passes a byte address and a bit number in [0, 7].
> +	 * On x86, update just that byte to avoid crossing a page boundary.
> +	 * Bound retries so a contended byte cannot hold up the worker.
> +	 */
> +	pagefault_disable();
> +	if (!user_access_begin(p, sizeof(*p)))
> +		goto out;
> +	unsafe_get_user(old, p, fault);
> +	do {
> +		if (unsafe_try_cmpxchg_user(p, &old, old | BIT(nr), fault)) {
> +			success = true;
> +			break;
> +		}
> +	} while (--retries);
> +fault:
> +	user_access_end();
> +out:
> +	pagefault_enable();
> +	return success;
> +}
> +#else
> +static bool set_bit_to_user_fast(int nr, void __user *addr)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_X86 */
> +
> +static int set_bit_to_user(int nr, void __user *addr)
> +{
> +	if (set_bit_to_user_fast(nr, addr))
> +		return 0;
> +	/* Resolve faults, or use the native bit layout on other architectures. */
> +	return set_bit_to_user_slow(nr, addr);
> +}
> +
>  static int log_write(void __user *log_base,
>  		     u64 write_address, u64 write_length)
>  {


Actually why do we need all the unsafe trickery, is there
a reason? below seems way simpler?

+static int set_bit_to_user(int nr, void __user *addr)
+{
+       u8 __user *p = addr;
+       u8 old;
+       int i, ret;
+
+       if (get_user(old, p))
+               return set_bit_to_user_slow(nr, addr);
+       for (i = 0; i < 4; i++) {
+               ret = __try_cmpxchg_user(p, &old, old | BIT(nr), fault);
+               if (!ret)
+                       return 0;
+       }
+       return set_bit_to_user_slow(nr, addr);
+}








> -- 
> 2.43.0

Re: [PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

[Michael S. Tsirkin]

On Thu, Nov 12, 2026 at 04:56:30PM +0800, zhaoguohan@kylinos.cn wrote:
> From: Khan Zhao <zhaoguohan@kylinos.cn>
> 
> set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
> with kmap_atomic(), sets one bit and then unpins the page and marks it
> dirty. This runs for every bit set in the dirty log, e.g. on each used
> ring update while logging is enabled for live migration.
> 
> log_write() always passes a byte address and a bit number in [0, 7], so
> the update can be done as a byte-sized cmpxchg on the user address. On
> x86, try that first using unsafe_try_cmpxchg_user() inside a
> user_access_begin()/user_access_end() section with page faults disabled.
> If the access faults (page not present, read-only or needing COW), or
> the cmpxchg loses to concurrent updates of the same byte four times in
> a row, fall back to the existing pin/kmap/set_bit path, which can fault
> the page in.
> 
> unsafe_try_cmpxchg_user() is only implemented on x86, so other
> architectures keep using the existing path.
> 
> On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
> workload with logging enabled went from 243.60 to 171.11 ns per
> descriptor, averaged over five alternating runs. This is the
> end-to-end cost per descriptor, not live migration performance.
> 
> Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
> ---
>  drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e29f154706da..9c3045504709 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
>  
> -/* TODO: This is really inefficient.  We need something like get_user()
> - * (instruction directly accesses the data, with an exception table entry
> - * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
> - */
> -static int set_bit_to_user(int nr, void __user *addr)
> +static int set_bit_to_user_slow(int nr, void __user *addr)
>  {
>  	unsigned long log = (unsigned long)addr;
>  	struct page *page;
> @@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_X86

Wait this is x86 specific anyway? So let's just implement it
properly, why do we need all these hacks?

I run a quick test on this in a VM, feel free to reuse:

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>



diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a0dd3c2b233e..3ed49c91a7434 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -594,6 +594,23 @@ _label:									\
 	__ret;								\
 							})
 
+#define set_bit_user(nr, addr)					\
+({									\
+	int __err = 0;							\
+	__uaccess_begin();						\
+	asm volatile("\n"						\
+		     "1: " LOCK_PREFIX "btsb %[bit], %[ptr]\n"		\
+		     "2:\n"						\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
+					   %[errout])			\
+		     : [errout] "=r" (__err),				\
+		       [ptr] "+m" (*(u8 __user *)(addr))		\
+		     : [bit] "r" ((unsigned int)(nr))			\
+		     : "memory", "cc");				\
+	__uaccess_end();						\
+	__err ? -EFAULT : 0;						\
+})
+
 /*
  * We want the unsafe accessors to always be inlined and use
  * the error labels - thus the macro games.

Re: [PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

[Michael S. Tsirkin]

On Thu, Nov 12, 2026 at 05:10:10AM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 12, 2026 at 04:56:30PM +0800, zhaoguohan@kylinos.cn wrote:
> > From: Khan Zhao <zhaoguohan@kylinos.cn>
> > 
> > set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
> > with kmap_atomic(), sets one bit and then unpins the page and marks it
> > dirty. This runs for every bit set in the dirty log, e.g. on each used
> > ring update while logging is enabled for live migration.
> > 
> > log_write() always passes a byte address and a bit number in [0, 7], so
> > the update can be done as a byte-sized cmpxchg on the user address. On
> > x86, try that first using unsafe_try_cmpxchg_user() inside a
> > user_access_begin()/user_access_end() section with page faults disabled.
> > If the access faults (page not present, read-only or needing COW), or
> > the cmpxchg loses to concurrent updates of the same byte four times in
> > a row, fall back to the existing pin/kmap/set_bit path, which can fault
> > the page in.
> > 
> > unsafe_try_cmpxchg_user() is only implemented on x86, so other
> > architectures keep using the existing path.
> > 
> > On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
> > workload with logging enabled went from 243.60 to 171.11 ns per
> > descriptor, averaged over five alternating runs. This is the
> > end-to-end cost per descriptor, not live migration performance.
> > 
> > Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
> > ---
> >  drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 45 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index e29f154706da..9c3045504709 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> >  }
> >  EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
> >  
> > -/* TODO: This is really inefficient.  We need something like get_user()
> > - * (instruction directly accesses the data, with an exception table entry
> > - * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
> > - */
> > -static int set_bit_to_user(int nr, void __user *addr)
> > +static int set_bit_to_user_slow(int nr, void __user *addr)
> >  {
> >  	unsigned long log = (unsigned long)addr;
> >  	struct page *page;
> > @@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
> >  	return 0;
> >  }
> >  
> > +#ifdef CONFIG_X86
> 
> Wait this is x86 specific anyway? So let's just implement it
> properly, why do we need all these hacks?
> 
> I run a quick test on this in a VM, feel free to reuse:
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 

sorry fat fingers. wrong file. this one seems to work.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a0dd3c2b233e..ba1e7c3f83c8e 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -594,6 +594,24 @@ _label:									\
 	__ret;								\
 							})
 
+/* set_bit() on a byte of user memory; returns -EFAULT on fault. */
+#define set_bit_user(nr, addr)					\
+({									\
+	int __err = 0;							\
+	__uaccess_begin();						\
+	asm volatile("\n"						\
+		     "1: " LOCK_PREFIX "orb %[mask], %[ptr]\n"		\
+		     "2:\n"						\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
+					   %[errout])			\
+		     : [errout] "=r" (__err),				\
+		       [ptr] "+m" (*(u8 __user *)(addr))		\
+		     : [mask] "q" ((unsigned char)BIT(nr))		\
+		     : "memory", "cc");				\
+	__uaccess_end();						\
+	__err ? -EFAULT : 0;						\
+})
+
 /*
  * We want the unsafe accessors to always be inlined and use
  * the error labels - thus the macro games.