Running out of room in the pool is routine for a device with a bounded one: vring_map_errno() reports it as the -ENOSPC a full queue reports, so the driver retries and carries on. The allocator must not log that at any level a working device prints, or an undersized region turns into a log flood. That leaves an operator nothing but unexplained throughput loss to go on.
swiotlb can afford dev_warn_ratelimited() because exhaustion there is a misconfiguration, and it exports io_tlb_used and io_tlb_used_hiwater through debugfs, which a log line cannot do for a sampled quantity. Follow it, plus one file swiotlb has no need for: dmb/pages pages the allocator can hand out dmb/areas pool areas the pool is divided into dmb/area_pages pages one pool area covers dmb/used_pages pages allocated now dmb/used_pages_hiwater the largest used_pages has been dmb/alloc_failed buffer mappings the pool had no room for used_pages_hiwater is the number to size a region against, because a burst that fills the pool between two samples of used_pages leaves no other trace. Writing 0 restarts the measurement from the occupancy now, so a peak never reads below the used_pages read alongside it. alloc_failed counts only the mapping path, so that a virtqueue area rejected during vring_alloc_queue_split()'s search for a size that fits does not make a correctly sized region look undersized. There is no sysfs file, because a sysfs attribute is ABI and nothing here should be. Every file and every counter behind one sits under CONFIG_VIRTIO_DEBUG, so a production build carries none of them. Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf <[email protected]> --- .../driver-api/virtio/virtio-dmb.rst | 52 ++++- drivers/virtio/virtio_dmb.c | 215 +++++++++++++++++- 2 files changed, 262 insertions(+), 5 deletions(-) diff --git a/Documentation/driver-api/virtio/virtio-dmb.rst b/Documentation/driver-api/virtio/virtio-dmb.rst index 4cd23e25901f..610db080dee5 100644 --- a/Documentation/driver-api/virtio/virtio-dmb.rst +++ b/Documentation/driver-api/virtio/virtio-dmb.rst @@ -255,8 +255,10 @@ The last area is short unless ``area_pages`` divides ``pages``. A region small enough for one area to cover behaves exactly as a single lock over the whole pool does, which includes every region up to 2 MiB on a 4 KiB page, and more than that on a guest with few possible CPUs. -Both values are printed at probe time, so a device implementer can read -back what a guest derived from the region it offered. +Both values are printed at probe time and are readable under +``CONFIG_VIRTIO_DEBUG`` in ``dmb/areas`` and ``dmb/area_pages``, so a +device implementer can read back what a guest derived from the region it +offered. A device cannot influence the count and should not try to: it is derived from the guest's page size, cacheline size and possible-CPU count, none of @@ -619,7 +621,51 @@ a guest with three or more possible CPUs; a guest with one or two derives fewer, larger areas from the same region. The claimed range also appears in ``/proc/iomem`` as ``virtio-dmb``, but only where the claim was granted, and the ``shmid`` -appears nowhere else at all. +appears nowhere else at all. The three counts appear again in +``dmb/pages``, ``dmb/areas`` and ``dmb/area_pages`` below. + +With ``CONFIG_VIRTIO_DEBUG`` the state of the region is also available +under the device's virtio debugfs directory, in ``dmb/``. The directory +exists only while the device has a region, so a device that did not +negotiate the feature has no ``dmb/`` at all. These files are +diagnostics and not ABI: their names, their contents and the one write +they accept may change or go away. + +``pages`` + how many pages the allocator can hand out. + +``areas`` + how many pool areas the pool is divided into. + +``area_pages`` + how many pages one pool area covers. ``areas`` is + ``ceil(pages / area_pages)``, so the last area covers fewer unless + ``area_pages`` divides ``pages``. Both are fixed when the region is + installed and are the same two values the probe-time message prints. + +``used_pages`` + how many of them are allocated. One counter maintained across all areas + rather than a sum of per-area figures read at different moments, so it + never reports a torn total; it is raised just outside the area lock, so a + read taken during a claim or a release can lag the bitmap by that claim. + +``used_pages_hiwater`` + the largest ``used_pages`` has been. This, rather than a sample of + ``used_pages``, is what a region should be sized against: a burst + that fills the region between two samples leaves no other trace. + Writing ``0`` restarts the measurement from the current occupancy. + +``alloc_failed`` + how many buffer mappings the pool had no room for. Because running + out of room is an ordinary condition it is not logged above debug + level, so this is the cheapest indication that a region is too small + for what is running on it: a non-zero value means the pool could not + fit a mapping the workload asked for, either because it is smaller + than the workload needs or because no single pool area held a long + enough run of free pages. A virtqueue area that did not fit is not + counted here, since it is logged instead, and neither is a mapping + refused for exceeding the per-mapping cap, which ``max_mapping_size`` + advertises. Notes for kernel code ===================== diff --git a/drivers/virtio/virtio_dmb.c b/drivers/virtio/virtio_dmb.c index 42126f928bc0..fe556d585f24 100644 --- a/drivers/virtio/virtio_dmb.c +++ b/drivers/virtio/virtio_dmb.c @@ -20,10 +20,12 @@ */ #include <linux/align.h> +#include <linux/atomic.h> #include <linux/bitmap.h> #include <linux/bits.h> #include <linux/cache.h> #include <linux/cpumask.h> +#include <linux/debugfs.h> #include <linux/dma-mapping.h> #include <linux/export.h> #include <linux/highmem.h> @@ -126,7 +128,12 @@ struct virtio_dmb_area { * @area_slots: slots one area covers, a power of two; the last area covers * fewer when @nslots is not a multiple of it * @area_shift: ilog2(@area_slots), so slot >> @area_shift names its area + * @total_used: slots allocated across every area, exact; CONFIG_VIRTIO_DEBUG + * @used_hiwater: the largest @total_used has been since the last reset through + * debugfs, or since init; CONFIG_VIRTIO_DEBUG + * @alloc_failed: buffer mappings the pool had no room for; CONFIG_VIRTIO_DEBUG * @shm_id: shared memory id the device reported for the region + * @debugfs_dir: directory holding this region's debugfs files */ struct virtio_dmb { struct virtio_device *vdev; @@ -145,7 +152,20 @@ struct virtio_dmb { unsigned int nareas; unsigned int area_slots; unsigned int area_shift; +#ifdef CONFIG_VIRTIO_DEBUG + /* + * Exact occupancy for the two debugfs files, kept outside the area + * locks. A production build has neither: summing the per-area counts + * would need every lock, and the hot path is what this file exists to + * make cheap. kernel/dma/swiotlb.c draws the same line at + * CONFIG_DEBUG_FS. + */ + atomic_long_t total_used; + atomic_long_t used_hiwater; + atomic_long_t alloc_failed; +#endif u16 shm_id; + struct dentry *debugfs_dir; }; /* First slot of area @i. */ @@ -210,6 +230,61 @@ static size_t virtio_dmb_max_mapping(const struct virtio_dmb *dmb) return max_t(size_t, min(eighth, half_area), PAGE_SIZE); } +#ifdef CONFIG_VIRTIO_DEBUG + +/* + * Exact global occupancy, kept outside every area lock. + * + * Summing the per-area counts would be imprecise, because no two of them are + * read under the same lock, and taking every lock to read one debugfs file + * would put a whole-pool serialisation back into a file whose only purpose is + * to observe. An atomic add-return instead yields a value @total_used + * genuinely held, so raising the high-water mark to the largest such value + * makes both figures exact rather than approximate: two racing claims each + * observe a distinct real total and the larger wins. + * + * This is kernel/dma/swiotlb.c's inc_used_and_hiwater()/dec_used() pair, for + * the same reason and with the same empty stubs when the option is off. + */ +static void virtio_dmb_inc_used(struct virtio_dmb *dmb, unsigned int nr) +{ + long old_hiwater, new_used; + + new_used = atomic_long_add_return(nr, &dmb->total_used); + old_hiwater = atomic_long_read(&dmb->used_hiwater); + do { + if (new_used <= old_hiwater) + break; + } while (!atomic_long_try_cmpxchg(&dmb->used_hiwater, &old_hiwater, + new_used)); +} + +static void virtio_dmb_dec_used(struct virtio_dmb *dmb, unsigned int nr) +{ + atomic_long_sub(nr, &dmb->total_used); +} + +static void virtio_dmb_inc_alloc_failed(struct virtio_dmb *dmb) +{ + atomic_long_inc(&dmb->alloc_failed); +} + +#else /* !CONFIG_VIRTIO_DEBUG */ + +static void virtio_dmb_inc_used(struct virtio_dmb *dmb, unsigned int nr) +{ +} + +static void virtio_dmb_dec_used(struct virtio_dmb *dmb, unsigned int nr) +{ +} + +static void virtio_dmb_inc_alloc_failed(struct virtio_dmb *dmb) +{ +} + +#endif /* CONFIG_VIRTIO_DEBUG */ + /* * Claim @nr contiguous slots from area @i, or -ENOMEM when that one area * cannot satisfy the request. Takes and drops that area's lock and touches @@ -254,6 +329,12 @@ static long virtio_dmb_area_claim(struct virtio_dmb *dmb, unsigned int i, spin_unlock_irqrestore(&area->lock, flags); + /* + * Outside the lock, which is where swiotlb_search_pool_area() does it + * too: it touches no area state, so holding one buys nothing. + */ + virtio_dmb_inc_used(dmb, nr); + return slot; not_found: @@ -271,7 +352,9 @@ static long virtio_dmb_area_claim(struct virtio_dmb *dmb, unsigned int i, * buffer, and repolls instead. Reporting it at any level a working device * would print would therefore be a log flood, and it is the only signal an * undersized region produces at all, so it is reported through dynamic debug - * where it costs nothing until somebody asks for it. + * where it costs nothing until somebody asks for it. The map_page() caller + * counts it as well, so that a debug build offers a sampled reader as well as + * a log. * * Next fit within one area, from a hint that advances past each claim and * rewinds to each release, beginning in the area belonging to the running CPU @@ -390,6 +473,18 @@ static void virtio_dmb_release(struct virtio_dmb *dmb, unsigned int slot, area->used -= nr; area->index = slot - virtio_dmb_area_base(dmb, a); + spin_unlock_irqrestore(&area->lock, flags); + + /* + * Outside the lock, and deliberately not below the label: the guard + * above rejects a range that is not wholly allocated, and only slots + * that were counted in are counted out, so the total tracks the bitmap + * rather than the caller's arithmetic. + */ + virtio_dmb_dec_used(dmb, nr); + + return; + out: spin_unlock_irqrestore(&area->lock, flags); } @@ -717,8 +812,22 @@ static dma_addr_t virtio_dmb_op_map_page(union virtio_map map, nr = virtio_dmb_slots(size); ret = virtio_dmb_claim(dmb, nr); - if (ret < 0) + if (ret < 0) { + /* + * Counted here rather than in virtio_dmb_claim(), which + * virtio_dmb_op_alloc() reaches as well. A virtqueue area + * that does not fit is a step of vring_alloc_queue_split()'s + * search for a size that does, so counting it would have a + * correctly sized region boot with a failure for every + * attempt but the last, in the one file whose purpose is to + * answer whether the region is too small for the traffic. A + * request over the per-mapping cap is not counted either: the + * cap is what max_mapping_size() advertises, so exceeding it + * is a caller bug rather than a property of the region. + */ + virtio_dmb_inc_alloc_failed(dmb); return DMA_MAPPING_ERROR; + } slot = ret; virtio_dmb_record(dmb, slot, nr, size, src); @@ -860,6 +969,105 @@ static const struct virtio_map_ops virtio_dmb_map_ops = { .max_mapping_size = virtio_dmb_op_max_mapping_size, }; +#ifdef CONFIG_VIRTIO_DEBUG + +static int virtio_dmb_used_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb = data; + + *val = atomic_long_read(&dmb->total_used); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_used_fops, virtio_dmb_used_get, NULL, + "%llu\n"); + +static int virtio_dmb_hiwater_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb = data; + + *val = atomic_long_read(&dmb->used_hiwater); + + return 0; +} + +static int virtio_dmb_hiwater_set(void *data, u64 val) +{ + struct virtio_dmb *dmb = data; + + /* Restarting the measurement is the only meaningful write. */ + if (val) + return -EINVAL; + + /* + * Restart from what is allocated now rather than from zero, so that + * the file never reports a peak below the occupancy it is read + * alongside. + */ + atomic_long_set(&dmb->used_hiwater, + atomic_long_read(&dmb->total_used)); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_hiwater_fops, virtio_dmb_hiwater_get, + virtio_dmb_hiwater_set, "%llu\n"); + +static int virtio_dmb_alloc_failed_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb = data; + + *val = atomic_long_read(&dmb->alloc_failed); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_alloc_failed_fops, + virtio_dmb_alloc_failed_get, NULL, "%llu\n"); + +/* + * The files go under the device's existing virtio debugfs directory, and exist + * only while the device has a region. They describe one, so their presence is + * also the answer to whether the device is using the feature. + * + * Every file is served through DEFINE_DEBUGFS_ATTRIBUTE, either directly or by + * a debugfs_create_*() helper that uses it, so each read takes a reference + * that debugfs_remove_recursive() waits for. That is what lets the caller + * free the state these files point at once the directory is gone. + */ +static void virtio_dmb_debugfs_init(struct virtio_dmb *dmb) +{ + struct dentry *dir; + + dir = debugfs_create_dir("dmb", dmb->vdev->debugfs_dir); + dmb->debugfs_dir = dir; + + debugfs_create_u32("pages", 0400, dir, &dmb->nslots); + debugfs_create_u32("areas", 0400, dir, &dmb->nareas); + debugfs_create_u32("area_pages", 0400, dir, &dmb->area_slots); + debugfs_create_file("used_pages", 0400, dir, dmb, + &virtio_dmb_used_fops); + debugfs_create_file("used_pages_hiwater", 0600, dir, dmb, + &virtio_dmb_hiwater_fops); + debugfs_create_file("alloc_failed", 0400, dir, dmb, + &virtio_dmb_alloc_failed_fops); +} + +static void virtio_dmb_debugfs_exit(struct virtio_dmb *dmb) +{ + debugfs_remove_recursive(dmb->debugfs_dir); +} + +#else /* !CONFIG_VIRTIO_DEBUG */ + +static void virtio_dmb_debugfs_init(struct virtio_dmb *dmb) +{ +} + +static void virtio_dmb_debugfs_exit(struct virtio_dmb *dmb) +{ +} + +#endif /* CONFIG_VIRTIO_DEBUG */ + /* * Whether the device still has virtqueues. vqs_list_lock is what protects * that list against a concurrent adder. No caller here can race one, because @@ -929,6 +1137,7 @@ void virtio_dmb_destroy(struct virtio_device *vdev) vdev->map = dmb->prev_map; vdev->vmap = dmb->prev_vmap; + virtio_dmb_debugfs_exit(dmb); memunmap(dmb->map_va); if (dmb->map_claimed) release_mem_region(dmb->map_phys, dmb->map_len); @@ -1278,6 +1487,8 @@ int virtio_dmb_init(struct virtio_device *vdev) dmb->area_slots = area_slots; dmb->area_shift = ilog2(area_slots); + virtio_dmb_debugfs_init(dmb); + /* Published last: until now nothing routes a mapping here. */ dmb->prev_map = vdev->map; dmb->prev_vmap = vdev->vmap;
