Re: cvs: pecl /apc apc_cache.c apc_lock.h apc_main.c apc_sma.c apc_sma.h config.m4 php_apc.c
| From: | Brian Shire | Date: | Thu, 28 Sep 2006 22:26:50 +0000 |
| Subject: | Re: cvs: pecl /apc apc_cache.c apc_lock.h apc_main.c apc_sma.c apc_sma.h config.m4 php_apc.c | ||
| References: | 1 2 | Groups: | php.apc.dev |
| Request: | Send a blank email to php-apc-dev+get-13@lists.php.net to get a copy of this message | ||
Yes, my patch doesn't currently address this situation. I haven't reviewed the linux/Documentation/robust-futex*.txt files in great detail yet. I'll review these in more detail and see if I can improve this situation.
-shire
On Sep 28, 2006, at 3:01 PM, Rasmus Lerdorf wrote:
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 UTCModified 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=uIndex: 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;#endiffor (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_MMAPapc_unmap(sma_shmaddrs[i], sma_segments[i]);#elseapc_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=uIndex: 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_SEMphp_info_print_table_row(2, "Locking type", "IPC Semaphore");+#elif APC_FUTEX_LOCKS+ php_info_print_table_row(2, "Locking type", "Linux Futex Locks");#elsephp_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_LOCKSadd_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);#elseadd_assoc_stringl(return_value, "locking_type", "file", sizeof("file"), 1);#endif