On Mon, 11 Nov 2019 15:55:05 +0000 (UTC) Christopher Lameter <cl@xxxxxxxxx> wrote:
> Regardless of the issue with memcgs allowing allocations from its
> kmalloc array during shutdown: This patch cleans things up and properly
> allocates the bitmap outside of the list_lock.
>
>
> [FIX] slub: Remove kmalloc under list_lock from list_slab_objects() V2
>
> V1->V2 : Properly handle CONFIG_SLUB_DEBUG. Handle bitmap free correctly.
>
> list_slab_objects() is called when a slab is destroyed and there are objects still left
> to list the objects in the syslog. This is a pretty rare event.
>
> And there it seems we take the list_lock and call kmalloc while holding that lock.
>
> Perform the allocation in free_partial() before the list_lock is taken.
No response here? It looks a lot simpler than the originally proposed
patch?
> --- linux.orig/mm/slub.c 2019-10-15 13:54:57.032655296 +0000
> +++ linux/mm/slub.c 2019-11-11 15:52:11.616397853 +0000
> @@ -3690,14 +3690,15 @@ error:
> }
>
> static void list_slab_objects(struct kmem_cache *s, struct page *page,
> - const char *text)
> + const char *text, unsigned long *map)
> {
> #ifdef CONFIG_SLUB_DEBUG
> void *addr = page_address(page);
> void *p;
> - unsigned long *map = bitmap_zalloc(page->objects, GFP_ATOMIC);
> +
> if (!map)
> return;
> +
> slab_err(s, page, text, s->name);
> slab_lock(page);
>
> @@ -3710,7 +3711,6 @@ static void list_slab_objects(struct kme
> }
> }
> slab_unlock(page);
> - bitmap_free(map);
> #endif
> }
>
> @@ -3723,6 +3723,11 @@ static void free_partial(struct kmem_cac
> {
> LIST_HEAD(discard);
> struct page *page, *h;
> + unsigned long *map = NULL;
> +
> +#ifdef CONFIG_SLUB_DEBUG
> + map = bitmap_alloc(oo_objects(s->max), GFP_KERNEL);
> +#endif
>
> BUG_ON(irqs_disabled());
> spin_lock_irq(&n->list_lock);
> @@ -3732,11 +3737,16 @@ static void free_partial(struct kmem_cac
> list_add(&page->slab_list, &discard);
> } else {
> list_slab_objects(s, page,
> - "Objects remaining in %s on __kmem_cache_shutdown()");
> + "Objects remaining in %s on __kmem_cache_shutdown()",
> + map);
> }
> }
> spin_unlock_irq(&n->list_lock);
>
> +#ifdef CONFIG_SLUB_DEBUG
> + bitmap_free(map);
> +#endif
> +
> list_for_each_entry_safe(page, h, &discard, slab_list)
> discard_slab(s, page);
> }
