[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [UNIKRAFT PATCH 00/18] Allocator statistics


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
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,

On Thu, Jan 7, 2021 at 6:21 PM Laurentiu Barbulescu <lrbarbulescu@xxxxxxxxx> wrote:
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,

On Fri, Nov 20, 2020 at 5:46 PM Simon Kuenzer <simon.kuenzer@xxxxxxxxx> wrote:
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

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(),
  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%)




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.