Re: [Xen-devel] [PATCH v3] xmalloc: add support for checking the pool integrity

[Mihai DonÈu <mdontu@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx>
> 
> ---
> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel