Skip to contentSkip to conversation

patch -- mempool buglet (?)

PATCH David Brownell 2 participants 2 messages Dec 31, 2002

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

David Brownell

author
#0 lore raw

I noticed this when reading the mempool code ... looked wrong to me, it was using kfree() not the de-allocator matching the allocation it just made. This is on a fault path that likely doesn't get much use.

Compiles, untested, "looks right".

- Dave

mempool.c renamed from mempool.c-dist +2 -2
@@ -142,14 +142,14 @@
 		element = pool->alloc(gfp_mask, pool->pool_data);
 		if (!element)
 			goto out;
 		spin_lock_irqsave(&pool->lock, flags);
 		if (pool->curr_nr < pool->min_nr)
 			add_element(pool, element);
-		else
-			kfree(element);		/* Raced */
+		else 	/* Raced */
+			pool->free(element, pool->pool_data);
 	}
 out_unlock:
 	spin_unlock_irqrestore(&pool->lock, flags);
 out:
 	return 0;
 }
#1 lore raw
Quoted 8 lines · David Brownell

I noticed this when reading the mempool code ... looked wrong to me, it was using kfree() not the de-allocator matching the allocation it just made. This is on a fault path that likely doesn't get much use.

Compiles, untested, "looks right".

Yup, thanks. We actually need to drop the pool->lock around the pool->free invokation to be consistent. I'll send in a fix.

mm/mempool.c renamed from mm/mempool.c~mempool_resize-fix +18 -10
@@ -87,7 +87,13 @@ mempool_t * mempool_create(int min_nr, m
 	}
 	return pool;
 }
+EXPORT_SYMBOL(mempool_create);
 
+/*
+ * mempool_resize is disabled for now, because it has no callers.  Feel free
+ * to turn it back on if needed.
+ */
+#if 0
 /**
  * mempool_resize - resize an existing memory pool
  * @pool:       pointer to the memory pool which was allocated via

@@ -143,16 +149,21 @@ int mempool_resize(mempool_t *pool, int 
 		if (!element)
 			goto out;
 		spin_lock_irqsave(&pool->lock, flags);
-		if (pool->curr_nr < pool->min_nr)
+		if (pool->curr_nr < pool->min_nr) {
 			add_element(pool, element);
-		else
-			kfree(element);		/* Raced */
+		} else {
+			spin_unlock_irqrestore(&pool->lock, flags);
+			pool->free(element, pool->pool_data);	/* Raced */
+			spin_lock_irqsave(&pool->lock, flags);
+		}
 	}
 out_unlock:
 	spin_unlock_irqrestore(&pool->lock, flags);
 out:
 	return 0;
 }
+EXPORT_SYMBOL(mempool_resize);
+#endif
 
 /**
  * mempool_destroy - deallocate a memory pool

@@ -169,6 +180,7 @@ void mempool_destroy(mempool_t *pool)
 		BUG();		/* There were outstanding elements */
 	free_pool(pool);
 }
+EXPORT_SYMBOL(mempool_destroy);
 
 /**
  * mempool_alloc - allocate an element from a specific memory pool

@@ -230,6 +242,7 @@ repeat_alloc:
 
 	goto repeat_alloc;
 }
+EXPORT_SYMBOL(mempool_alloc);
 
 /**
  * mempool_free - return an element to the pool.

@@ -255,6 +268,7 @@ void mempool_free(void *element, mempool
 	}
 	pool->free(element, pool->pool_data);
 }
+EXPORT_SYMBOL(mempool_free);
 
 /*
  * A commonly used alloc and free fn.

@@ -264,17 +278,11 @@ void *mempool_alloc_slab(int gfp_mask, v
 	kmem_cache_t *mem = (kmem_cache_t *) pool_data;
 	return kmem_cache_alloc(mem, gfp_mask);
 }
+EXPORT_SYMBOL(mempool_alloc_slab);
 
 void mempool_free_slab(void *element, void *pool_data)
 {
 	kmem_cache_t *mem = (kmem_cache_t *) pool_data;
 	kmem_cache_free(mem, element);
 }
-
-EXPORT_SYMBOL(mempool_create);
-EXPORT_SYMBOL(mempool_resize);
-EXPORT_SYMBOL(mempool_destroy);
-EXPORT_SYMBOL(mempool_alloc);
-EXPORT_SYMBOL(mempool_free);
-EXPORT_SYMBOL(mempool_alloc_slab);
 EXPORT_SYMBOL(mempool_free_slab);

_