Skip to contentSkip to conversation

mm: slub: Always flush the delayed empty slubs in flush_all()

PATCH Applied Kevin Hao 3 participants 3 messages May 4, 2020

Subsystems (2)
Memory management Slab allocator
lore
Share & tools
lore

Kevin Hao

author
#0 lore raw

PATCH 1/1 mm: slub: Always flush the delayed empty slubs in flush_all()

After commit f0b231101c94 ("mm/SLUB: delay giving back empty slubs to IRQ enabled regions"), when the free_slab() is invoked with the IRQ disabled, the empty slubs are moved to a per-CPU list and will be freed after IRQ enabled later. But in the current codes, there is a check to see if there really has the cpu slub on a specific cpu before flushing the delayed empty slubs, this may cause a reference of already released kmem_cache in a scenario like below:

	cpu 0				cpu 1
  kmem_cache_destroy()
    flush_all()
                         --->IPI       flush_cpu_slab()
                                         flush_slab()
                                           deactivate_slab()
                                             discard_slab()
                                               free_slab()
                                             c->page = NULL;
      for_each_online_cpu(cpu)
        if (!has_cpu_slab(1, s))
          continue
        this skip to flush the delayed
        empty slub released by cpu1
    kmem_cache_free(kmem_cache, s)

                                       kmalloc()
                                         __slab_alloc()
                                            free_delayed()
                                            __free_slab()
                                            reference to released kmem_cache
Fixes f0b231101c94 ("mm/SLUB: delay giving back empty slubs to IRQ enabled regions")
Signed-off-by Kevin Hao <[email protected]>
---
 mm/slub.c | 3 ---
 1 file changed, 3 deletions(-)
mm/slub.c +0 -3
index 15c194ff16e6..83b29bf71fd0 100644
@@ -2402,9 +2402,6 @@ static void flush_all(struct kmem_cache *s)
 	for_each_online_cpu(cpu) {
 		struct slub_free_list *f;
 
-		if (!has_cpu_slab(cpu, s))
-			continue;
-
 		f = &per_cpu(slub_free_list, cpu);
 		raw_spin_lock_irq(&f->lock);
 		list_splice_init(&f->list, &tofree);
Signature · 1 line
2.26.0
Applied
#1 lore raw
Quoted 1 line · Kevin Hao

After commit f0b231101c94 ("mm/SLUB: delay giving back empty slubs to

…

Quoted 2 lines
Fixes f0b231101c94 ("mm/SLUB: delay giving back empty slubs to IRQ enabled regions")
Signed-off-by Kevin Hao <[email protected]>

Applied, thanks.

Sebastian

reviewer Acked-by on v1 1/1
#2 lore raw
In reply to Kevin Hao
Quoted 29 lines · Kevin Hao

After commit f0b231101c94 ("mm/SLUB: delay giving back empty slubs to IRQ enabled regions"), when the free_slab() is invoked with the IRQ disabled, the empty slubs are moved to a per-CPU list and will be freed after IRQ enabled later. But in the current codes, there is a check to see if there really has the cpu slub on a specific cpu before flushing the delayed empty slubs, this may cause a reference of already released kmem_cache in a scenario like below:

	cpu 0				cpu 1
  kmem_cache_destroy()
    flush_all()
                         --->IPI       flush_cpu_slab()
                                         flush_slab()
                                           deactivate_slab()
                                             discard_slab()
                                               free_slab()
                                             c->page = NULL;
      for_each_online_cpu(cpu)
        if (!has_cpu_slab(1, s))
          continue
        this skip to flush the delayed
        empty slub released by cpu1
    kmem_cache_free(kmem_cache, s)

                                       kmalloc()
                                         __slab_alloc()
                                            free_delayed()
                                            __free_slab()
                                            reference to released kmem_cache
Quoted 2 lines
Fixes f0b231101c94 ("mm/SLUB: delay giving back empty slubs to IRQ enabled regions")
Signed-off-by Kevin Hao <[email protected]>
Acked-by David Rientjes <[email protected]>