Re: [patch v2 4/4] mm, mempool: poison elements backed by page allocator

[Andrey Ryabinin <ryabinin.a.a@gmail.com>]

2015-03-25 2:10 GMT+03:00 David Rientjes <rientjes@google.com>:

...
>
> +
> +static void check_element(mempool_t *pool, void *element)
> +{
> +       /* Mempools backed by slab allocator */
> +       if (pool->free == mempool_free_slab || pool->free == mempool_kfree)
> +               __check_element(pool, element, ksize(element));
> +
> +       /* Mempools backed by page allocator */
> +       if (pool->free == mempool_free_pages) {
> +               int order = (int)(long)pool->pool_data;
> +               void *addr = page_address(element);
> +
> +               __check_element(pool, addr, 1UL << (PAGE_SHIFT + order));
>         }
>  }
>
> -static void poison_slab_element(mempool_t *pool, void *element)
> +static void __poison_element(void *element, size_t size)
>  {
> -       if (pool->alloc == mempool_alloc_slab ||
> -           pool->alloc == mempool_kmalloc) {
> -               size_t size = ksize(element);
> -               u8 *obj = element;
> +       u8 *obj = element;
> +
> +       memset(obj, POISON_FREE, size - 1);
> +       obj[size - 1] = POISON_END;
> +}
> +
> +static void poison_element(mempool_t *pool, void *element)
> +{
> +       /* Mempools backed by slab allocator */
> +       if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> +               __poison_element(element, ksize(element));
> +
> +       /* Mempools backed by page allocator */
> +       if (pool->alloc == mempool_alloc_pages) {
> +               int order = (int)(long)pool->pool_data;
> +               void *addr = page_address(element);
>
> -               memset(obj, POISON_FREE, size - 1);
> -               obj[size - 1] = POISON_END;
> +               __poison_element(addr, 1UL << (PAGE_SHIFT + order));

I think, it would be better to use kernel_map_pages() here and in
check_element().
This implies that poison_element()/check_element() has to be moved out of
CONFIG_DEBUG_SLAB || CONFIG_SLUB_DEBUG_ON ifdef (keeping only slab
poisoning under this ifdef).
After these changes it might be a good idea to rename
poison_element()/check_element()
to something like debug_add_element()/debug_remove_element() respectively.