[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [UNIKRAFT PATCH 10/18] lib/ukalloc: Allocator statistics
I have 3 inline comments for this patch: This commit introduces an implementation for the configuration option
`LIBUKALLOC_IFSTATS`. When enabled, allocators keep allocation statistics,
like number of current allocations, maximum memory usage due to
allocations, or how often ENOMEM was returned.
The statistics are kept on the `uk_alloc` struct while the instrumentation
of an allocator is minimal: An allocator has only to be modified with
the following macros:
- `uk_alloc_stats_count_alloc()`, `uk_alloc_stats_count_palloc()`
- `uk_alloc_stats_count_free()`, `uk_alloc_stats_count_pfree()`
- `uk_alloc_stats_count_enomem()`, `uk_alloc_stats_count_penomem()`
Current statistics can be queried with `uk_alloc_stats()`.
Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
lib/ukalloc/Config.uk | 6 ++
lib/ukalloc/Makefile.uk | 1 +
lib/ukalloc/exportsyms.uk | 1 +
lib/ukalloc/include/uk/alloc.h | 29 ++++++++
lib/ukalloc/include/uk/alloc_impl.h | 108 ++++++++++++++++++++++++++++
lib/ukalloc/stats.c | 47 ++++++++++++
6 files changed, 192 insertions(+)
create mode 100644 lib/ukalloc/stats.c
diff --git a/lib/ukalloc/Config.uk b/lib/ukalloc/Config.uk
index 59a6aa78..fc64a13b 100644
--- a/lib/ukalloc/Config.uk
+++ b/lib/ukalloc/Config.uk
@@ -10,4 +10,10 @@ if LIBUKALLOC
default n
help
Provide helpers for allocators defining exclusively malloc and free
+
+ config LIBUKALLOC_IFSTATS
+ bool "Allocator statistics interface"
+ default n
+ help
+ Provide interfaces for querying allocator statistics.
endif
diff --git a/lib/ukalloc/Makefile.uk b/lib/ukalloc/Makefile.uk
index 30636462..d56aabb6 100644
--- a/lib/ukalloc/Makefile.uk
+++ b/lib/ukalloc/Makefile.uk
@@ -4,3 +4,4 @@ CINCLUDES-$(CONFIG_LIBUKALLOC) += -I$(LIBUKALLOC_BASE)/include
CXXINCLUDES-$(CONFIG_LIBUKALLOC) += -I$(LIBUKALLOC_BASE)/include
LIBUKALLOC_SRCS-y += $(LIBUKALLOC_BASE)/alloc.c
+LIBUKALLOC_SRCS-$(CONFIG_LIBUKALLOC_IFSTATS) += $(LIBUKALLOC_BASE)/stats.c
diff --git a/lib/ukalloc/exportsyms.uk b/lib/ukalloc/exportsyms.uk
index 7c25a70c..48de908c 100644
--- a/lib/ukalloc/exportsyms.uk
+++ b/lib/ukalloc/exportsyms.uk
@@ -20,3 +20,4 @@ uk_alloc_pavail_compat
uk_alloc_availmem_total
uk_alloc_pavail_total
_uk_alloc_head
+uk_alloc_stats
diff --git a/lib/ukalloc/include/uk/alloc.h b/lib/ukalloc/include/uk/alloc.h
index fc19551e..59155d8c 100644
--- a/lib/ukalloc/include/uk/alloc.h
+++ b/lib/ukalloc/include/uk/alloc.h
@@ -71,6 +71,24 @@ typedef ssize_t (*uk_alloc_availmem_func_t)
typedef long (*uk_alloc_pavail_func_t)
(struct uk_alloc *a);
+#if CONFIG_LIBUKALLOC_IFSTATS
Honestly I don't know if there is a convention for this, but I found a bit confusing the struct `uk_alloc_stats` and the function with the same name, `uk_alloc_stats`.
+struct uk_alloc_stats {
+ size_t last_alloc_size; /* size of the last allocation */
+ size_t max_alloc_size; /* biggest satisfied allocation size */
+ size_t min_alloc_size; /* smallest satisfied allocation size */
+
+ uint64_t tot_nb_allocs; /* total number of satisfied allocations */
+ uint64_t tot_nb_frees; /* total number of satisfied free operations */
+ uint64_t cur_nb_allocs; /* current number of active allocations */
+ uint64_t max_nb_allocs; /* maximum number of active allocations */
+
+ size_t cur_mem_use; /* current used memory by allocations */
+ size_t max_mem_use; /* maximum amount of memory used by allocations */
+
+ uint64_t nb_enomem; /* number of times failing allocation requests */
+};
+#endif /* CONFIG_LIBUKALLOC_IFSTATS */
+
struct uk_alloc {
/* memory allocation */
uk_alloc_malloc_func_t malloc;
@@ -96,6 +114,10 @@ struct uk_alloc {
/* optional interface */
uk_alloc_addmem_func_t addmem;
+#if CONFIG_LIBUKALLOC_IFSTATS
+ struct uk_alloc_stats _stats;
+#endif
+
/* internal */
struct uk_alloc *next;
int8_t priv[];
@@ -283,6 +305,13 @@ size_t uk_alloc_availmem_total(void);
unsigned long uk_alloc_pavail_total(void);
+#if CONFIG_LIBUKALLOC_IFSTATS
+/*
+ * Memory allocation statistics
+ */
+void uk_alloc_stats(struct uk_alloc *a, struct uk_alloc_stats *dst);
+#endif /* CONFIG_LIBUKALLOC_IFSTATS */
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/ukalloc/include/uk/alloc_impl.h b/lib/ukalloc/include/uk/alloc_impl.h
index 4811558e..9ef6df13 100644
--- a/lib/ukalloc/include/uk/alloc_impl.h
+++ b/lib/ukalloc/include/uk/alloc_impl.h
@@ -40,6 +40,7 @@
#define __UK_ALLOC_IMPL_H__
#include <uk/alloc.h>
+#include <uk/preempt.h>
#ifdef __cplusplus
extern "C" {
@@ -81,6 +82,110 @@ void uk_pfree_compat(struct uk_alloc *a, void *ptr, unsigned long num_pages);
long uk_alloc_pavail_compat(struct uk_alloc *a);
long uk_alloc_pmaxalloc_compat(struct uk_alloc *a);
+#if CONFIG_LIBUKALLOC_IFSTATS Another thing is that `<uk/preempt.h>` is used only under `#if CONFIG_LIBUKALLOC_IFSTATS`, so we can include it here too.
+#include <string.h>
+
+#if CONFIG_LIBUKALLOC_IFSTATS_GLOBAL
+extern struct uk_alloc_stats _uk_alloc_stats_global;
+#endif /* CONFIG_LIBUKALLOC_IFSTATS_GLOBAL */
+
Just for consistency reasons, I don't know why the `__uk_alloc_stats_refresh_minmax` function has an extra `_` in the prefix, compared to the other functions.
+static inline void __uk_alloc_stats_refresh_minmax(struct uk_alloc_stats *stats)
+{
+ if (stats->cur_nb_allocs > stats->max_nb_allocs)
+ stats->max_nb_allocs = stats->cur_nb_allocs;
+
+ if (stats->cur_mem_use > stats->max_mem_use)
+ stats->max_mem_use = stats->cur_mem_use;
+
+ if (stats->last_alloc_size) {
+ /* Force storing the minimum allocation size
+ * with the first allocation request
+ */
+ if ((stats->tot_nb_allocs == 1)
+ || (stats->last_alloc_size < stats->min_alloc_size))
+ stats->min_alloc_size = stats->last_alloc_size;
+ if (stats->last_alloc_size > stats->max_alloc_size)
+ stats->max_alloc_size = stats->last_alloc_size;
+ }
+}
+
+static inline void _uk_alloc_stats_count_alloc(struct uk_alloc_stats *stats,
+ void *ptr, size_t size)
+{
+ /* TODO: SMP safety */
+ uk_preempt_disable();
+ if (likely(ptr)) {
+ stats->tot_nb_allocs++;
+
+ stats->cur_nb_allocs++;
+ stats->cur_mem_use += size;
+ stats->last_alloc_size = size;
+ __uk_alloc_stats_refresh_minmax(stats);
+ } else {
+ stats->nb_enomem++;
+ }
+ uk_preempt_enable();
+}
+
+static inline void _uk_alloc_stats_count_free(struct uk_alloc_stats *stats,
+ void *ptr, size_t size)
+{
+ uk_preempt_disable();
+ if (likely(ptr)) {
+ stats->tot_nb_frees++;
+
+ stats->cur_nb_allocs--;
+ stats->cur_mem_use -= size;
+ __uk_alloc_stats_refresh_minmax(stats);
+ }
+ uk_preempt_enable();
+}
+
+/*
+ * The following macros should be used to instrument an allocator for
+ * statistics:
+ */
+/* NOTE: If ptr is NULL, an ENOMEM event is counted */
+#define uk_alloc_stats_count_alloc(a, ptr, size) \
+ do { \
+ _uk_alloc_stats_count_alloc(&((a)->_stats), \
+ (ptr), (size)); \
+ } while (0)
+#define uk_alloc_stats_count_palloc(a, ptr, num_pages) \
+ uk_alloc_stats_count_alloc((a), (ptr), \
+ ((__sz) (num_pages)) << __PAGE_SHIFT)
+
+#define uk_alloc_stats_count_enomem(a, size) \
+ uk_alloc_stats_count_alloc((a), NULL, (size))
+#define uk_alloc_stats_count_penomem(a, num_pages) \
+ uk_alloc_stats_count_enomem((a), \
+ ((__sz) (num_pages)) << __PAGE_SHIFT)
+
+/* Note: if ptr is NULL, nothing is counted */
+#define uk_alloc_stats_count_free(a, ptr, freed_size) \
+ do { \
+ _uk_alloc_stats_count_free(&((a)->_stats), \
+ (ptr), (freed_size)); \
+ } while (0)
+#define uk_alloc_stats_count_pfree(a, ptr, num_pages) \
+ uk_alloc_stats_count_free((a), (ptr), \
+ ((__sz) (num_pages)) << __PAGE_SHIFT)
+
+#define uk_alloc_stats_reset(a) \
+ do { \
+ memset(&(a)->_stats, 0, sizeof((a)->_stats)); \
+ } while (0)
+
+#else /* !CONFIG_LIBUKALLOC_IFSTATS */
+#define uk_alloc_stats_count_alloc(a, ptr, size) do {} while (0)
+#define uk_alloc_stats_count_palloc(a, ptr, num_pages) do {} while (0)
+#define uk_alloc_stats_count_enomem(a, size) do {} while (0)
+#define uk_alloc_stats_count_penomem(a, num_pages) do {} while (0)
+#define uk_alloc_stats_count_free(a, ptr, freed_size) do {} while (0)
+#define uk_alloc_stats_count_pfree(a, ptr, num_pages) do {} while (0)
+#define uk_alloc_stats_reset(a) do {} while (0)
+#endif /* !CONFIG_LIBUKALLOC_IFSTATS */
+
/* Shortcut for doing a registration of an allocator that does not implement
* palloc() or pfree()
*/
@@ -104,6 +209,7 @@ long uk_alloc_pmaxalloc_compat(struct uk_alloc *a);
? uk_alloc_pmaxalloc_compat : NULL; \
(a)->addmem = (addmem_f); \
\
+ uk_alloc_stats_reset((a)); \
uk_alloc_register((a)); \
} while (0)
@@ -129,6 +235,7 @@ long uk_alloc_pmaxalloc_compat(struct uk_alloc *a);
? uk_alloc_pmaxalloc_compat : NULL; \
(a)->addmem = (addmem_f); \
\
+ uk_alloc_stats_reset((a)); \
uk_alloc_register((a)); \
} while (0)
#endif
@@ -155,6 +262,7 @@ long uk_alloc_pmaxalloc_compat(struct uk_alloc *a);
? uk_alloc_maxalloc_ifpages : NULL; \
(a)->addmem = (addmem_func); \
\
+ uk_alloc_stats_reset((a)); \
uk_alloc_register((a)); \
} while (0)
diff --git a/lib/ukalloc/stats.c b/lib/ukalloc/stats.c
new file mode 100644
index 00000000..26dcc405
--- /dev/null
+++ b/lib/ukalloc/stats.c
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
+ *
+ * Copyright (c) 2020, NEC Laboratories Europe GmbH, NEC Corporation.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <string.h>
+#include <uk/preempt.h>
+#include <uk/alloc_impl.h>
+
+void uk_alloc_stats(struct uk_alloc *a,
+ struct uk_alloc_stats *dst)
+{
+ UK_ASSERT(a);
+ UK_ASSERT(dst);
+
+ uk_preempt_disable();
+ memcpy(dst, &a->_stats, sizeof(*dst));
+ uk_preempt_enable();
+}
--
2.20.1
|
