Re: [PECL-CVS] cvs: pecl /apc apc_cache.c apc_lock.h apc_main.c apc_sma.c apc_sma.h config.m4 php_apc.c
[email protected] (Rasmus Lerdorf) Thu, 28 Sep 2006 15:01:05 -0700
| Newsgroups | php.apc.dev |
|---|---|
| Message-ID | <[email protected]> |
What always stopped me from implementing futex support was the lack of
owner-death cleanup which will lead to deadlocks. Ingo and company came
up with a way to handle this not too long ago by keeping a list of held
futex locks in userspace, but it always seemed rather flaky to me and
you have to coordinate the lock release with the list of locks and avoid
a race condition where the process that just got the lock dies before it
has a chance to update the list of held locks. If the two get out of
sync we are back in a deadlock. This is something you get for free
(well at the cost of some performance) with the other locking mechanisms
and looking through your patch I don't see anything that addresses this.
-Rasmus
Brian Shire wrote:
> shire Thu Sep 28 21:22:09 2006 UTC
>
> Modified files:
> /pecl/apc apc_cache.c apc_lock.h apc_main.c apc_sma.c apc_sma.h
> config.m4 php_apc.c
> Log:
> Added linux futex lock support (--enable-apc-futex)
> Added arch directory with architecture specific code.
> Moved lock values to shared memory segments.
> Removed the Safety Net code added by rasmus but ifdef'd out (incl. apc_sma_lock function as it caused difficulty with the lock move).
>
>
>
>
> ------------------------------------------------------------------------
>
> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_cache.c?r1=3.124&r2=3.125&diff_format=u
> Index: pecl/apc/apc_cache.c
> diff -u pecl/apc/apc_cache.c:3.124 pecl/apc/apc_cache.c:3.125
> --- pecl/apc/apc_cache.c:3.124 Sat Sep 23 22:38:06 2006
> +++ pecl/apc/apc_cache.c Thu Sep 28 21:22:09 2006
> @@ -28,7 +28,7 @@
>
> */
>
> -/* $Id: apc_cache.c,v 3.124 2006/09/23 22:38:06 iliaa Exp $ */
> +/* $Id: apc_cache.c,v 3.125 2006/09/28 21:22:09 shire Exp $ */
>
> #include "apc_cache.h"
> #include "apc_lock.h"
> @@ -43,10 +43,10 @@
>
> /* {{{ locking macros */
> #define CREATE_LOCK apc_lck_create(NULL, 0, 1)
> -#define DESTROY_LOCK(c) apc_lck_destroy(c->lock)
> -#define LOCK(c) { HANDLE_BLOCK_INTERRUPTIONS(); apc_lck_lock(c->lock); }
> -#define RDLOCK(c) { HANDLE_BLOCK_INTERRUPTIONS(); apc_lck_rdlock(c->lock); }
> -#define UNLOCK(c) { apc_lck_unlock(c->lock); HANDLE_UNBLOCK_INTERRUPTIONS(); }
> +#define DESTROY_LOCK(c) apc_lck_destroy(c->header->lock)
> +#define LOCK(c) { HANDLE_BLOCK_INTERRUPTIONS(); apc_lck_lock(c->header->lock); }
> +#define RDLOCK(c) { HANDLE_BLOCK_INTERRUPTIONS(); apc_lck_rdlock(c->header->lock); }
> +#define UNLOCK(c) { apc_lck_unlock(c->header->lock); HANDLE_UNBLOCK_INTERRUPTIONS(); }
> /* }}} */
>
> /* {{{ struct definition: slot_t */
> @@ -66,6 +66,8 @@
> Any values that must be shared among processes should go in here. */
> typedef struct header_t header_t;
> struct header_t {
> + int lock; /* read/write lock (exclusive blocking cache lock) */
> + int wrlock; /* write lock (non-blocking used to prevent cache slams) */
> int num_hits; /* total successful hits in cache */
> int num_misses; /* total unsuccessful hits in cache */
> int num_inserts; /* total successful inserts in cache */
> @@ -86,8 +88,6 @@
> int num_slots; /* number of slots in cache */
> int gc_ttl; /* maximum time on GC list for a slot */
> int ttl; /* if slot is needed and entry's access time is older than this ttl, remove it */
> - int lock; /* read/write lock (exclusive blocking cache lock) */
> - int wrlock; /* write lock (non-blocking used to prevent cache slams) */
> };
> /* }}} */
>
> @@ -298,9 +298,9 @@
> cache->num_slots = num_slots;
> cache->gc_ttl = gc_ttl;
> cache->ttl = ttl;
> - cache->lock = CREATE_LOCK;
> + cache->header->lock = CREATE_LOCK;
> #if NONBLOCKING_LOCK_AVAILABLE
> - cache->wrlock = CREATE_LOCK;
> + cache->header->wrlock = CREATE_LOCK;
> #endif
> for (i = 0; i < num_slots; i++) {
> cache->slots[i] = NULL;
> @@ -1068,14 +1068,14 @@
> /* {{{ apc_cache_write_lock */
> zend_bool apc_cache_write_lock(apc_cache_t* cache)
> {
> - return apc_lck_nb_lock(cache->wrlock);
> + return apc_lck_nb_lock(cache->header->wrlock);
> }
> /* }}} */
>
> /* {{{ apc_cache_write_unlock */
> void apc_cache_write_unlock(apc_cache_t* cache)
> {
> - apc_lck_unlock(cache->wrlock);
> + apc_lck_unlock(cache->header->wrlock);
> }
> /* }}} */
> #endif
> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_lock.h?r1=3.14&r2=3.15&diff_format=u
> Index: pecl/apc/apc_lock.h
> diff -u pecl/apc/apc_lock.h:3.14 pecl/apc/apc_lock.h:3.15
> --- pecl/apc/apc_lock.h:3.14 Sun Sep 24 17:27:32 2006
> +++ pecl/apc/apc_lock.h Thu Sep 28 21:22:09 2006
> @@ -26,7 +26,7 @@
>
> */
>
> -/* $Id: apc_lock.h,v 3.14 2006/09/24 17:27:32 iliaa Exp $ */
> +/* $Id: apc_lock.h,v 3.15 2006/09/28 21:22:09 shire Exp $ */
>
> #ifndef APC_LOCK
> #define APC_LOCK
> @@ -54,6 +54,14 @@
> #define apc_lck_lock(a) apc_sem_lock(a)
> #define apc_lck_rdlock(a) apc_sem_lock(a)
> #define apc_lck_unlock(a) apc_sem_unlock(a)
> +#elif defined(APC_FUTEX_LOCKS)
> +#define NONBLOCKING_LOCK_AVAILABLE 1
> +#define apc_lck_create(a,b,c) apc_futex_create()
> +#define apc_lck_destroy(a) apc_futex_destroy(&a)
> +#define apc_lck_lock(a) apc_futex_lock(&a)
> +#define apc_lck_nb_lock(a) apc_futex_nonblocking_lock(&a)
> +#define apc_lck_rdlock(a) apc_futex_lock(&a)
> +#define apc_lck_unlock(a) apc_futex_unlock(&a)
> #else
> #define RDLOCK_AVAILABLE 1
> #ifdef PHP_WIN32
> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_main.c?r1=3.83&r2=3.84&diff_format=u
> Index: pecl/apc/apc_main.c
> diff -u pecl/apc/apc_main.c:3.83 pecl/apc/apc_main.c:3.84
> --- pecl/apc/apc_main.c:3.83 Mon Sep 4 23:56:39 2006
> +++ pecl/apc/apc_main.c Thu Sep 28 21:22:09 2006
> @@ -28,7 +28,7 @@
>
> */
>
> -/* $Id: apc_main.c,v 3.83 2006/09/04 23:56:39 rasmus Exp $ */
> +/* $Id: apc_main.c,v 3.84 2006/09/28 21:22:09 shire Exp $ */
>
> #include "apc_php.h"
> #include "apc_main.h"
> @@ -584,11 +584,6 @@
> (apc_cache_entry_t*) apc_stack_pop(APCG(cache_stack));
> apc_cache_release(apc_cache, cache_entry);
> }
> - /* Safety net */
> -#if 0
> - apc_sma_unlock();
> - apc_cache_unlock(apc_cache);
> -#endif
> }
> /* }}} */
>
> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_sma.c?r1=1.53&r2=1.54&diff_format=u
> Index: pecl/apc/apc_sma.c
> diff -u pecl/apc/apc_sma.c:1.53 pecl/apc/apc_sma.c:1.54
> --- pecl/apc/apc_sma.c:1.53 Fri Jun 9 23:41:22 2006
> +++ pecl/apc/apc_sma.c Thu Sep 28 21:22:09 2006
> @@ -26,7 +26,7 @@
>
> */
>
> -/* $Id: apc_sma.c,v 1.53 2006/06/09 23:41:22 rasmus Exp $ */
> +/* $Id: apc_sma.c,v 1.54 2006/09/28 21:22:09 shire Exp $ */
>
> #include "apc_sma.h"
> #include "apc.h"
> @@ -53,10 +53,10 @@
> static int* sma_segments; /* array of shm segment ids */
> static void** sma_shmaddrs; /* array of shm segment addresses */
> static int sma_lastseg = 0; /* index of MRU segment */
> -static int sma_lock; /* sempahore to serialize access */
>
> typedef struct header_t header_t;
> struct header_t {
> + int sma_lock; /* segment lock, MUST BE ALIGNED for futex locks */
> size_t segsize; /* size of entire segment */
> size_t avail; /* bytes available (not necessarily contiguous) */
> size_t nfoffset; /* start next fit search from this offset */
> @@ -286,8 +286,6 @@
> sma_segments = (int*) apc_emalloc(sma_numseg*sizeof(int));
> sma_shmaddrs = (void**) apc_emalloc(sma_numseg*sizeof(void*));
>
> - sma_lock = apc_lck_create(NULL, 0, 1);
> -
> for (i = 0; i < sma_numseg; i++) {
> header_t* header;
> block_t* block;
> @@ -304,6 +302,7 @@
> shmaddr = sma_shmaddrs[i];
>
> header = (header_t*) shmaddr;
> + header->sma_lock = apc_lck_create(NULL, 0, 1);
> header->segsize = sma_segsize;
> header->avail = sma_segsize - sizeof(header_t) - sizeof(block_t) -
> alignword(sizeof(int));
> @@ -332,13 +331,13 @@
> assert(sma_initialized);
>
> for (i = 0; i < sma_numseg; i++) {
> + apc_lck_destroy(((header_t*)sma_shmaddrs[i])->sma_lock);
> #if APC_MMAP
> apc_unmap(sma_shmaddrs[i], sma_segments[i]);
> #else
> apc_shm_detach(sma_shmaddrs[i]);
> #endif
> }
> - apc_lck_destroy(sma_lock);
> sma_initialized = 0;
> apc_efree(sma_segments);
> apc_efree(sma_shmaddrs);
> @@ -353,17 +352,19 @@
>
> TSRMLS_FETCH();
> assert(sma_initialized);
> - LOCK(sma_lock);
> + LOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
>
> off = sma_allocate(sma_shmaddrs[sma_lastseg], n);
> if (off != -1) {
> void* p = (void *)(((char *)(sma_shmaddrs[sma_lastseg])) + off);
> if (APCG(mem_size_ptr) != NULL) { *(APCG(mem_size_ptr)) += n; }
> - UNLOCK(sma_lock);
> + UNLOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
> return p;
> }
> + UNLOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
>
> for (i = 0; i < sma_numseg; i++) {
> + LOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> if (i == sma_lastseg) {
> continue;
> }
> @@ -371,13 +372,13 @@
> if (off != -1) {
> void* p = (void *)(((char *)(sma_shmaddrs[i])) + off);
> if (APCG(mem_size_ptr) != NULL) { *(APCG(mem_size_ptr)) += n; }
> - UNLOCK(sma_lock);
> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> sma_lastseg = i;
> return p;
> }
> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> }
>
> - UNLOCK(sma_lock);
> return NULL;
> }
> /* }}} */
> @@ -417,20 +418,20 @@
> }
>
> assert(sma_initialized);
> - LOCK(sma_lock);
>
> for (i = 0; i < sma_numseg; i++) {
> + LOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> size_t d_size = (size_t)((char *)p - (char *)(sma_shmaddrs[i]));
> if (p >= sma_shmaddrs[i] && d_size < sma_segsize) {
> sma_deallocate(sma_shmaddrs[i], d_size);
> if (APCG(mem_size_ptr) != NULL) { *(APCG(mem_size_ptr)) -= d_size; }
> - UNLOCK(sma_lock);
> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> return;
> }
> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> }
>
> apc_eprint("apc_sma_free: could not locate address %p", p);
> - UNLOCK(sma_lock);
> }
> /* }}} */
>
> @@ -454,10 +455,9 @@
> info->list[i] = NULL;
> }
>
> - RDLOCK(sma_lock);
> -
> /* For each segment */
> for (i = 0; i < sma_numseg; i++) {
> + RDLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> char* shmaddr = sma_shmaddrs[i];
> block_t* prv = BLOCKAT(sizeof(header_t));
>
> @@ -475,9 +475,9 @@
>
> prv = cur;
> }
> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
> }
>
> - UNLOCK(sma_lock);
> return info;
> }
> /* }}} */
> @@ -520,12 +520,6 @@
> return header->adist;
> }
> #endif
> -/* {{{ apc_sma_unlock */
> -void apc_sma_unlock()
> -{
> - UNLOCK(sma_lock);
> -}
> -/* }}} */
>
> #if 0
> /* {{{ apc_sma_check_integrity */
> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_sma.h?r1=1.14&r2=1.15&diff_format=u
> Index: pecl/apc/apc_sma.h
> diff -u pecl/apc/apc_sma.h:1.14 pecl/apc/apc_sma.h:1.15
> --- pecl/apc/apc_sma.h:1.14 Sun Mar 12 00:31:45 2006
> +++ pecl/apc/apc_sma.h Thu Sep 28 21:22:09 2006
> @@ -25,7 +25,7 @@
>
> */
>
> -/* $Id: apc_sma.h,v 1.14 2006/03/12 00:31:45 rasmus Exp $ */
> +/* $Id: apc_sma.h,v 1.15 2006/09/28 21:22:09 shire Exp $ */
>
> #ifndef APC_SMA_H
> #define APC_SMA_H
> @@ -42,7 +42,6 @@
> extern void* apc_sma_realloc(void* p, size_t size);
> extern char* apc_sma_strdup(const char *s);
> extern void apc_sma_free(void* p);
> -extern void apc_sma_unlock();
> #if ALLOC_DISTRIBUTION
> extern size_t *apc_sma_get_alloc_distribution();
> #endif
> http://cvs.php.net/viewvc.cgi/pecl/apc/config.m4?r1=3.15&r2=3.16&diff_format=u
> Index: pecl/apc/config.m4
> diff -u pecl/apc/config.m4:3.15 pecl/apc/config.m4:3.16
> --- pecl/apc/config.m4:3.15 Sat Sep 9 23:51:15 2006
> +++ pecl/apc/config.m4 Thu Sep 28 21:22:09 2006
> @@ -1,5 +1,5 @@
> dnl
> -dnl $Id: config.m4,v 3.15 2006/09/09 23:51:15 rasmus Exp $
> +dnl $Id: config.m4,v 3.16 2006/09/28 21:22:09 shire Exp $
> dnl
>
> AC_MSG_CHECKING(whether apc needs to get compiler flags from apxs)
> @@ -67,9 +67,27 @@
> AC_MSG_RESULT(no)
> ])
>
> +AC_MSG_CHECKING(Checking whether we should use futex locking)
> +AC_ARG_ENABLE(apc-futex,
> +[ --enable-apc-futex
> + Enable linux futex based locks ],
> +[
> + PHP_APC_FUTEX=$enableval
> + AC_MSG_RESULT($enableval)
> +],
> +[
> + PHP_APC_FUTEX=no
> + AC_MSG_RESULT(no)
> +])
> +
> +if test "$PHP_APC_FUTEX" != "no"; then
> + AC_CHECK_HEADER(linux/futex.h, , [ AC_MSG_ERROR([futex.h not found. Please verify you that are running a 2.5 or older linux kernel and that futex support is enabled.]); ] )
> +fi
> +
> if test "$PHP_APC" != "no"; then
> test "$PHP_APC_MMAP" != "no" && AC_DEFINE(APC_MMAP, 1, [ ])
> test "$PHP_APC_SEM" != "no" && AC_DEFINE(APC_SEM_LOCKS, 1, [ ])
> + test "$PHP_APC_FUTEX" != "no" && AC_DEFINE(APC_FUTEX_LOCKS, 1, [ ])
>
> AC_CACHE_CHECK(for union semun, php_cv_semun,
> [
> @@ -100,6 +118,7 @@
> apc_pair.c \
> apc_sem.c \
> apc_shm.c \
> + apc_futex.c \
> apc_sma.c \
> apc_stack.c \
> apc_zend.c \
> http://cvs.php.net/viewvc.cgi/pecl/apc/php_apc.c?r1=3.113&r2=3.114&diff_format=u
> Index: pecl/apc/php_apc.c
> diff -u pecl/apc/php_apc.c:3.113 pecl/apc/php_apc.c:3.114
> --- pecl/apc/php_apc.c:3.113 Thu Sep 21 11:52:16 2006
> +++ pecl/apc/php_apc.c Thu Sep 28 21:22:09 2006
> @@ -26,7 +26,7 @@
>
> */
>
> -/* $Id: php_apc.c,v 3.113 2006/09/21 11:52:16 gopalv Exp $ */
> +/* $Id: php_apc.c,v 3.114 2006/09/28 21:22:09 shire Exp $ */
>
> #include "apc_zend.h"
> #include "apc_cache.h"
> @@ -161,10 +161,12 @@
> #endif
> #if APC_SEM
> php_info_print_table_row(2, "Locking type", "IPC Semaphore");
> +#elif APC_FUTEX_LOCKS
> + php_info_print_table_row(2, "Locking type", "Linux Futex Locks");
> #else
> php_info_print_table_row(2, "Locking type", "File Locks");
> #endif
> - php_info_print_table_row(2, "Revision", "$Revision: 3.113 $");
> + php_info_print_table_row(2, "Revision", "$Revision: 3.114 $");
> php_info_print_table_row(2, "Build Date", __DATE__ " " __TIME__);
> php_info_print_table_end();
> DISPLAY_INI_ENTRIES();
> @@ -289,6 +291,8 @@
> #endif
> #if APC_SEM_LOCKS
> add_assoc_stringl(return_value, "locking_type", "IPC semaphore", sizeof("IPC semaphore"), 1);
> +#elif APC_FUTEX_LOCKS
> + add_assoc_stringl(return_value, "locking_type", "Linux Futex", sizeof("Linux Futex"), 1);
> #else
> add_assoc_stringl(return_value, "locking_type", "file", sizeof("file"), 1);
> #endif
>
>