MINOR: memory: Change the flush_lock to a spinlock, and don't get it in alloc.
The flush_lock was introduced, mostly to be sure that pool_gc() will never
dereference a pointer that has been free'd. __pool_get_first() was acquiring
the lock to, the fear was that otherwise that pointer could get free'd later,
and then pool_gc() would attempt to dereference it. However, that can not
happen, because the only functions that can free a pointer, when using
lockless pools, are pool_gc() and pool_flush(), and as long as those two
are mutually exclusive, nobody will be able to free the pointer while
pool_gc() attempts to access it.
So change the flush_lock to a spinlock, and don't bother acquire/release
it in __pool_get_first(), that way callers of __pool_get_first() won't have
to wait while the pool is flushed. The worst that can happen is we call
__pool_refill_alloc() while the pool is getting flushed, and memory can
get allocated just to be free'd.
This may help with github issue #552
This may be backported to 2.1, 2.0 and 1.9.
diff --git a/include/common/memory.h b/include/common/memory.h
index 5433a73..44a9dcb 100644
--- a/include/common/memory.h
+++ b/include/common/memory.h
@@ -78,7 +78,7 @@
void **free_list;
#ifdef CONFIG_HAP_LOCKLESS_POOLS
uintptr_t seq;
- HA_RWLOCK_T flush_lock;
+ HA_SPINLOCK_T flush_lock;
#else
__decl_hathreads(HA_SPINLOCK_T lock); /* the spin lock */
#endif
@@ -222,19 +222,15 @@
cmp.seq = pool->seq;
__ha_barrier_load();
- HA_RWLOCK_RDLOCK(POOL_LOCK, &pool->flush_lock);
cmp.free_list = pool->free_list;
do {
- if (cmp.free_list == NULL) {
- HA_RWLOCK_RDUNLOCK(POOL_LOCK, &pool->flush_lock);
+ if (cmp.free_list == NULL)
return NULL;
- }
new.seq = cmp.seq + 1;
__ha_barrier_load();
new.free_list = *POOL_LINK(pool, cmp.free_list);
} while (HA_ATOMIC_DWCAS((void *)&pool->free_list, (void *)&cmp, (void *)&new) == 0);
__ha_barrier_atomic_store();
- HA_RWLOCK_RDUNLOCK(POOL_LOCK, &pool->flush_lock);
_HA_ATOMIC_ADD(&pool->used, 1);
#ifdef DEBUG_MEMORY_POOLS