* [PATCH v3] xmalloc: add support for checking the pool integrity
@ 2014-12-10 12:13 Mihai Donțu
2014-12-10 12:27 ` Mihai Donțu
2014-12-10 12:56 ` Jan Beulich
0 siblings, 2 replies; 3+ messages in thread
From: Mihai Donțu @ 2014-12-10 12:13 UTC (permalink / raw)
To: xen-devel; +Cc: keir, ian.jackson, ian.campbell, jbeulich, tim
Implemented xmem_pool_check(), xmem_pool_check_locked() and
xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
Signed-off-by: Mihai Donțu <mdontu@bitdefender.com>
---
Changes since v2:
- print the name of the corrupted pool
- adjusted the messages to better fit within 80 columns
- minor style change (a ? a : b -> a ?: b)
Changes since v1:
- fixed the codingstyle
- swaped _locked/_unlocked naming
- reworked __xmem_pool_check_locked() a bit
- used bool_t where appropriate
- made xmem_pool_check() take a pool argument which can be NULL
---
xen/common/xmalloc_tlsf.c | 117 +++++++++++++++++++++++++++++++++++++++++++++-
xen/include/xen/xmalloc.h | 7 +++
2 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index a5769c9..1dea137 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -120,9 +120,118 @@ struct xmem_pool {
char name[MAX_POOL_NAME_LEN];
};
+static struct xmem_pool *xenpool;
+
+static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
+
/*
* Helping functions
*/
+#ifndef NDEBUG
+static bool_t xmem_pool_check_size(const struct xmem_pool *pool, int fl, int sl)
+{
+ const struct bhdr *b = pool->matrix[fl][sl];
+
+ while ( b )
+ {
+ int __fl;
+ int __sl;
+
+ MAPPING_INSERT(b->size, &__fl, &__sl);
+ if ( __fl != fl || __sl != sl )
+ {
+ printk(XENLOG_ERR
+ "xmem_pool: %s: misplaced block %p:%u ({%d,%d} -> {%d,%d})\n",
+ pool->name, b, b->size, fl, sl, __fl, __sl);
+ return 0;
+ }
+ b = b->ptr.free_ptr.next;
+ }
+ return 1;
+}
+
+/*
+ * This function must be called from a context where pool->lock is
+ * already acquired.
+ *
+ * Returns true if the pool has been corrupted, false otherwise
+ */
+#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__, __LINE__, pool)
+static bool_t __xmem_pool_check_locked(const char *file, int line, const struct xmem_pool *pool)
+{
+ int i;
+ static bool_t once = 1;
+
+ if ( !once )
+ goto out;
+ for ( i = 0; i < REAL_FLI; i++ )
+ {
+ int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
+
+ if ( fl >= 0 )
+ {
+ int j;
+
+ if ( !pool->sl_bitmap[fl] )
+ {
+ printk(XENLOG_ERR
+ "xmem_pool: %s: the TLSF bitmap is corrupted (non-empty FL with empty SL)\n",
+ pool->name);
+ __warn(file, line);
+ once = 0;
+ break;
+ }
+ for ( j = 0; j < MAX_SLI; j++ )
+ {
+ int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
+
+ if ( sl < 0 )
+ continue;
+ if ( !pool->matrix[fl][sl] )
+ {
+ printk(XENLOG_ERR
+ "xmem_pool: %s: the TLSF bitmap is corrupted (matrix[%d][%d] is NULL)\n",
+ pool->name, fl, sl);
+ __warn(file, line);
+ once = 0;
+ break;
+ }
+ if ( !xmem_pool_check_size(pool, fl, sl) )
+ {
+ printk(XENLOG_ERR "xmem_pool: %s: the TLSF chunk matrix is corrupted\n",
+ pool->name);
+ __warn(file, line);
+ once = 0;
+ break;
+ }
+ }
+ if ( !once )
+ break;
+ }
+ }
+out:
+ return !once;
+}
+
+#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__, __LINE__, pool)
+static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct xmem_pool *pool)
+{
+ bool_t oops;
+
+ spin_lock(&pool->lock);
+ oops = __xmem_pool_check_locked(file, line, pool);
+ spin_unlock(&pool->lock);
+ return oops;
+}
+
+bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
+{
+ return __xmem_pool_check_unlocked(file, line, pool ?: xenpool);
+}
+#else
+#define xmem_pool_check_locked(pool) ((void)(pool))
+#define xmem_pool_check_unlocked(pool) ((void)(pool))
+#endif
/**
* Returns indexes (fl, sl) of the list used to serve request of size r
@@ -381,6 +490,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
int fl, sl;
unsigned long tmp_size;
+ xmem_pool_check_unlocked(pool);
+
if ( pool->init_region == NULL )
{
if ( (region = pool->get_mem(pool->init_size)) == NULL )
@@ -442,11 +553,13 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
+ xmem_pool_check_locked(pool);
spin_unlock(&pool->lock);
return (void *)b->ptr.buffer;
/* Failed alloc */
out_locked:
+ xmem_pool_check_locked(pool);
spin_unlock(&pool->lock);
out:
@@ -464,6 +577,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
b = (struct bhdr *)((char *) ptr - BHDR_OVERHEAD);
spin_lock(&pool->lock);
+ xmem_pool_check_locked(pool);
b->size |= FREE_BLOCK;
pool->used_size -= (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
b->ptr.free_ptr = (struct free_ptr) { NULL, NULL};
@@ -500,6 +614,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
tmp_b->size |= PREV_FREE;
tmp_b->prev_hdr = b;
out:
+ xmem_pool_check_locked(pool);
spin_unlock(&pool->lock);
}
@@ -512,8 +627,6 @@ int xmem_pool_maxalloc(struct xmem_pool *pool)
* Glue for xmalloc().
*/
-static struct xmem_pool *xenpool;
-
static void *xmalloc_pool_get(unsigned long size)
{
ASSERT(size == PAGE_SIZE);
diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
index 24a99ac..ad48930 100644
--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -123,4 +123,11 @@ unsigned long xmem_pool_get_used_size(struct xmem_pool *pool);
*/
unsigned long xmem_pool_get_total_size(struct xmem_pool *pool);
+#ifndef NDEBUG
+#define xmem_pool_check(pool) __xmem_pool_check(__FILE__, __LINE__, pool)
+bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool);
+#else
+#define xmem_pool_check(pool) ((void)0)
+#endif
+
#endif /* __XMALLOC_H__ */
--
2.2.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] xmalloc: add support for checking the pool integrity
2014-12-10 12:13 [PATCH v3] xmalloc: add support for checking the pool integrity Mihai Donțu
@ 2014-12-10 12:27 ` Mihai Donțu
2014-12-10 12:56 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Mihai Donțu @ 2014-12-10 12:27 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, keir, ian.campbell, jbeulich, tim
On Wed, 10 Dec 2014 14:13:58 +0200 Mihai Donțu wrote:
> Implemented xmem_pool_check(), xmem_pool_check_locked() and
> xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
>
> Signed-off-by: Mihai Donțu <mdontu@bitdefender.com>
>
> ---
> Changes since v2:
> - print the name of the corrupted pool
> - adjusted the messages to better fit within 80 columns
> - minor style change (a ? a : b -> a ?: b)
>
> Changes since v1:
> - fixed the codingstyle
> - swaped _locked/_unlocked naming
> - reworked __xmem_pool_check_locked() a bit
> - used bool_t where appropriate
> - made xmem_pool_check() take a pool argument which can be NULL
> ---
> xen/common/xmalloc_tlsf.c | 117 +++++++++++++++++++++++++++++++++++++++++++++-
> xen/include/xen/xmalloc.h | 7 +++
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index a5769c9..1dea137 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -120,9 +120,118 @@ struct xmem_pool {
> char name[MAX_POOL_NAME_LEN];
> };
>
> +static struct xmem_pool *xenpool;
> +
> +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> +
> /*
> * Helping functions
> */
> +#ifndef NDEBUG
> +static bool_t xmem_pool_check_size(const struct xmem_pool *pool, int fl, int sl)
> +{
> + const struct bhdr *b = pool->matrix[fl][sl];
> +
> + while ( b )
> + {
> + int __fl;
> + int __sl;
> +
> + MAPPING_INSERT(b->size, &__fl, &__sl);
> + if ( __fl != fl || __sl != sl )
> + {
> + printk(XENLOG_ERR
> + "xmem_pool: %s: misplaced block %p:%u ({%d,%d} -> {%d,%d})\n",
> + pool->name, b, b->size, fl, sl, __fl, __sl);
> + return 0;
> + }
> + b = b->ptr.free_ptr.next;
> + }
> + return 1;
> +}
> +
> +/*
> + * This function must be called from a context where pool->lock is
> + * already acquired.
> + *
> + * Returns true if the pool has been corrupted, false otherwise
> + */
> +#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__, __LINE__, pool)
> +static bool_t __xmem_pool_check_locked(const char *file, int line, const struct xmem_pool *pool)
> +{
> + int i;
> + static bool_t once = 1;
> +
> + if ( !once )
> + goto out;
> + for ( i = 0; i < REAL_FLI; i++ )
> + {
> + int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
> +
> + if ( fl >= 0 )
> + {
> + int j;
> +
> + if ( !pool->sl_bitmap[fl] )
> + {
> + printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted (non-empty FL with empty SL)\n",
> + pool->name);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + for ( j = 0; j < MAX_SLI; j++ )
> + {
> + int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
> +
> + if ( sl < 0 )
> + continue;
> + if ( !pool->matrix[fl][sl] )
> + {
> + printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted (matrix[%d][%d] is NULL)\n",
> + pool->name, fl, sl);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + if ( !xmem_pool_check_size(pool, fl, sl) )
> + {
> + printk(XENLOG_ERR "xmem_pool: %s: the TLSF chunk matrix is corrupted\n",
> + pool->name);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + }
> + if ( !once )
> + break;
> + }
> + }
> +out:
> + return !once;
> +}
> +
> +#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__, __LINE__, pool)
> +static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct xmem_pool *pool)
> +{
> + bool_t oops;
> +
> + spin_lock(&pool->lock);
> + oops = __xmem_pool_check_locked(file, line, pool);
> + spin_unlock(&pool->lock);
> + return oops;
> +}
> +
> +bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
> +{
> + return __xmem_pool_check_unlocked(file, line, pool ?: xenpool);
> +}
> +#else
> +#define xmem_pool_check_locked(pool) ((void)(pool))
> +#define xmem_pool_check_unlocked(pool) ((void)(pool))
> +#endif
>
> /**
> * Returns indexes (fl, sl) of the list used to serve request of size r
> @@ -381,6 +490,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
> int fl, sl;
> unsigned long tmp_size;
>
> + xmem_pool_check_unlocked(pool);
> +
> if ( pool->init_region == NULL )
> {
> if ( (region = pool->get_mem(pool->init_size)) == NULL )
> @@ -442,11 +553,13 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
>
> pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
>
> + xmem_pool_check_locked(pool);
> spin_unlock(&pool->lock);
> return (void *)b->ptr.buffer;
>
> /* Failed alloc */
> out_locked:
> + xmem_pool_check_locked(pool);
> spin_unlock(&pool->lock);
>
> out:
> @@ -464,6 +577,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
> b = (struct bhdr *)((char *) ptr - BHDR_OVERHEAD);
>
> spin_lock(&pool->lock);
> + xmem_pool_check_locked(pool);
> b->size |= FREE_BLOCK;
> pool->used_size -= (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
> b->ptr.free_ptr = (struct free_ptr) { NULL, NULL};
> @@ -500,6 +614,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
> tmp_b->size |= PREV_FREE;
> tmp_b->prev_hdr = b;
> out:
> + xmem_pool_check_locked(pool);
> spin_unlock(&pool->lock);
> }
>
> @@ -512,8 +627,6 @@ int xmem_pool_maxalloc(struct xmem_pool *pool)
> * Glue for xmalloc().
> */
>
> -static struct xmem_pool *xenpool;
> -
> static void *xmalloc_pool_get(unsigned long size)
> {
> ASSERT(size == PAGE_SIZE);
> diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
> index 24a99ac..ad48930 100644
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -123,4 +123,11 @@ unsigned long xmem_pool_get_used_size(struct xmem_pool *pool);
> */
> unsigned long xmem_pool_get_total_size(struct xmem_pool *pool);
>
> +#ifndef NDEBUG
> +#define xmem_pool_check(pool) __xmem_pool_check(__FILE__, __LINE__, pool)
> +bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool);
> +#else
> +#define xmem_pool_check(pool) ((void)0)
> +#endif
> +
> #endif /* __XMALLOC_H__ */
(should've created a series)
This patch depends on:
http://lists.xenproject.org/archives/html/xen-devel/2014-12/msg01135.html
Than you,
--
Mihai DONȚU
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] xmalloc: add support for checking the pool integrity
2014-12-10 12:13 [PATCH v3] xmalloc: add support for checking the pool integrity Mihai Donțu
2014-12-10 12:27 ` Mihai Donțu
@ 2014-12-10 12:56 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2014-12-10 12:56 UTC (permalink / raw)
To: Mihai Donțu; +Cc: keir, tim, ian.jackson, ian.campbell, xen-devel
>>> On 10.12.14 at 13:13, <mdontu@bitdefender.com> wrote:
> +#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__, __LINE__, pool)
> +static bool_t __xmem_pool_check_locked(const char *file, int line, const struct xmem_pool *pool)
Long lines.
> +{
> + int i;
unsigned int
> + static bool_t once = 1;
> +
> + if ( !once )
> + goto out;
> + for ( i = 0; i < REAL_FLI; i++ )
> + {
> + int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
> +
> + if ( fl >= 0 )
> + {
> + int j;
unsigned int
> +
> + if ( !pool->sl_bitmap[fl] )
> + {
> + printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted (non-empty FL with empty SL)\n",
For the sake of brevity: "...: TLSF bitmap corrupt (...)\n" (please follow
advice given - as for an earlier message - throughout a patch).
> + pool->name);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + for ( j = 0; j < MAX_SLI; j++ )
> + {
> + int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
> +
> + if ( sl < 0 )
> + continue;
> + if ( !pool->matrix[fl][sl] )
> + {
> + printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted (matrix[%d][%d] is NULL)\n",
> + pool->name, fl, sl);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + if ( !xmem_pool_check_size(pool, fl, sl) )
> + {
> + printk(XENLOG_ERR "xmem_pool: %s: the TLSF chunk matrix is corrupted\n",
> + pool->name);
> + __warn(file, line);
> + once = 0;
> + break;
> + }
> + }
> + if ( !once )
> + break;
> + }
> + }
> +out:
Label should be indented by one space; whether a label is warranted
here is questionable though.
I thought I gave all these comments on v2 already, but it looks like
I accidentally lost them while reducing quoted text.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-10 12:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-10 12:13 [PATCH v3] xmalloc: add support for checking the pool integrity Mihai Donțu
2014-12-10 12:27 ` Mihai Donțu
2014-12-10 12:56 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.