[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [UNIKRAFT PATCH 00/18] Allocator statistics
Hi! I'm back with the review for the 2nd part of this patch series. The first thing is that, unfortunately, this series can no longer be applied because of removing the `THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.` line from: `lib/ukalloc/include/uk/alloc.h`, `lib/ukalloc/include/uk/alloc_impl.h` and `lib/nolibc/malloc.c`. Looks good overall, but I have a few more comments for the 10th and 17th patch. I will leave them there. Best wishes, Laurentiu
Hi Simon,
I was delegated to review only a part of this patch series (patches from 1 to 9). Great work overall, but I have some comments which I will leave inline at the corresponding patches(1, 5, 6 and 7).
Best wishes, Laurentiu
This patch series introduces allocator statistics. With minimal
instrumentation, allocators can report counters on allocation
and freeing events that happened.
For example, the following could be queried per allocator:
last_alloc_size: 230 B /* size of the last allocation */
max_alloc_size: 4096 B /* biggest satisfied allocation size */
min_alloc_size: 12 B /* smallest satisfied allocation size */
tot_nb_allocs: 75 /* total number of satisfied allocations */
tot_nb_frees: 22 /* total number of satisfied free operations */
cur_nb_allocs: 53 /* current number of active allocations */
max_nb_allocs: 53 /* maximum number of active allocations */
max_mem_use: 218023 B /* maximum amount of memory that was in use */
cur_mem_use: 217088 B /* current amount of memory that is in use */
nb_enomem: 0 /* number of times failing allocation requests */
Note: The meaning of `CONFIG_IFSTAT` changes with this patch series.
The configuration option originally raised wrong expectations while it
it just provided an API to query the amount of available memory from
an allocator. `CONFIG_IFSTAT` is now enabling these detailed allocator
statistics.
Note: Since this patch series changes some details on the internal ukalloc
API (`<uk/alloc_impl.h>`), external memory allocators (like TLSF, mimalloc)
need to be adopted as well. For retrieving statistics from them, they also
need to be instrumented to count alloc and free events.
The first four commits are doing minor corrections of the API headers.
Afterwards, the statistics API is introduced. Additionally, the last
commits introduce a one consolidated global allocator statistics, as
well as a per library allocator statistics. `lib/nolibc` had to
instrumented in order to support per-library statistics properly.
Similar to `nolibc`, `newlib` and `musl` need to be instrumented, too to
properly support per-library statistics for libc allocation interfaces:
malloc(), calloc(), free(), etc.
Simon Kuenzer (18):
lib/ukalloc: Correct BSD license header
lib/ukalloc: Move ifpages comment
lib/ukalloc: `extern C {` at the beginning of <uk/alloc.h>
lib/ukalloc: Move `uk_zalloc()` after `uk_calloc()`
lib/ukalloc: Introduce `uk_alloc_maxalloc()` and make
`uk_alloc_availmem()` available
lib/ukalloc: Introduce `uk_alloc_pavail()`, `uk_alloc_pmaxalloc()`
lib/ukalloc: Wrappers for "fee memory" interfaces
lib/ukalloc: Iterator helper for allocators
lib/ukalloc: Introduce `uk_alloc_availmem_total(),
`uk_alloc_pavail_total()`
lib/ukalloc: Allocator statistics
lib/ukalloc: Global statistics
lib/ukallocbbuddy: Instrumentation for statistics
lib/ukallocregion: Internal functions as `static`
lib/ukallocregion: Instrumentation for statistics
lib/ukalloc: Per-library allocation statistics
lib/ukalloc: Iterator for per-library statistics
lib/ukalloc: Use Unikraft internal types
lib/nolibc: Enable per-library allocator statistics
lib/nolibc/Makefile.uk | 1 -
lib/nolibc/include/stdlib.h | 39 ++-
lib/ukalloc/Config.uk | 29 ++-
lib/ukalloc/Makefile.uk | 5 +
lib/ukalloc/alloc.c | 225 ++++++++++------
lib/ukalloc/exportsyms.uk | 9 +
lib/ukalloc/include/uk/alloc.h | 174 +++++++++----
lib/ukalloc/include/uk/alloc_impl.h | 179 +++++++++++--
lib/ukalloc/libstats.c | 311 +++++++++++++++++++++++
lib/ukalloc/libstats.ld | 9 +
lib/ukalloc/libstats.localsyms.uk | 2 +
lib/{nolibc/malloc.c => ukalloc/stats.c} | 56 ++--
lib/ukallocbbuddy/bbuddy.c | 50 +++-
lib/ukallocregion/region.c | 54 +++-
plat/common/include/pci/pci_bus.h | 2 +
plat/common/memory.c | 2 +
plat/xen/gnttab.c | 1 +
plat/xen/memory.c | 2 +
plat/xen/x86/gnttab.c | 1 +
19 files changed, 952 insertions(+), 199 deletions(-)
create mode 100644 lib/ukalloc/libstats.c
create mode 100644 lib/ukalloc/libstats.ld
create mode 100644 lib/ukalloc/libstats.localsyms.uk
rename lib/{nolibc/malloc.c => ukalloc/stats.c} (62%)
--
2.20.1
|