usrloc: use recursive locks for location hash table

- allow recursive access to the same slot
- reported by Olle E. Johansson, GH #577

diff --git a/modules/usrloc/hslot.c b/modules/usrloc/hslot.c
index 592ec7e..f6441a7 100644 (file)
--- a/modules/usrloc/hslot.c
+++ b/modules/usrloc/hslot.c
@@ -29,113 +29,23 @@
 
 #include "hslot.h"
 
-/*! number of locks */
-int ul_locks_no=4;
-/*! global list of locks */
-gen_lock_set_t* ul_locks=0;
-
-
-/*!
- * \brief Initialize locks for the hash table
- * \return 0 on success, -1 on failure
- */
-int ul_init_locks(void)
-{
-       int i;
-       i = ul_locks_no;
-       do {
-               if ((( ul_locks=lock_set_alloc(i))!=0)&&
-                               (lock_set_init(ul_locks)!=0))
-               {
-                       ul_locks_no = i;
-                       LM_INFO("locks array size %d\n", ul_locks_no);
-                       return 0;
-
-               }
-               if (ul_locks){
-                       lock_set_dealloc(ul_locks);
-                       ul_locks=0;
-               }
-               i--;
-               if(i==0)
-               {
-                       LM_ERR("failed to allocate locks\n");
-                       return -1;
-               }
-       } while (1);
-}
-
-
-/*!
- * \brief Unlock all locks on the list
- */
-void ul_unlock_locks(void)
-{
-       unsigned int i;
-
-       if (ul_locks==0)
-               return;
-
-       for (i=0;i<ul_locks_no;i++) {
-#ifdef GEN_LOCK_T_PREFERED
-               lock_release(&ul_locks->locks[i]);
-#else
-               ul_release_idx(i);
-#endif
-       };
-}
-
-
-/*!
- * \brief Destroy all locks on the list
- */
-void ul_destroy_locks(void)
-{
-       if (ul_locks !=0){
-               lock_set_destroy(ul_locks);
-               lock_set_dealloc(ul_locks);
-       };
-}
-
-#ifndef GEN_LOCK_T_PREFERED
-/*!
- * \brief Lock a lock with a certain index
- * \param idx lock index
- */
-void ul_lock_idx(int idx)
-{
-       lock_set_get(ul_locks, idx);
-}
-
-
-/*!
- * \brief Release a lock with a certain index
- * \param idx lock index
- */
-void ul_release_idx(int idx)
-{
-       lock_set_release(ul_locks, idx);
-}
-#endif
-
 /*!
  * \brief Initialize cache slot structure
  * \param _d domain for the hash slot
  * \param _s hash slot
  * \param n used to get the slot number (modulo number or locks)
  */
-void init_slot(struct udomain* _d, hslot_t* _s, int n)
+int init_slot(struct udomain* _d, hslot_t* _s, int n)
 {
        _s->n = 0;
        _s->first = 0;
        _s->last = 0;
        _s->d = _d;
-
-#ifdef GEN_LOCK_T_PREFERED
-       _s->lock = &ul_locks->locks[n%ul_locks_no];
-#else
-       _s->lockidx = n%ul_locks_no;
-#endif
+       if(rec_lock_init(&_s->rlock)==NULL) {
+               LM_ERR("failed to initialize the slock (%d)\n", n);
+               return -1;
+       }
+       return 0;
 }
 
 
@@ -146,14 +56,15 @@ void init_slot(struct udomain* _d, hslot_t* _s, int n)
 void deinit_slot(hslot_t* _s)
 {
        struct urecord* ptr;
-       
-            /* Remove all elements */
+
+       /* Remove all elements */
        while(_s->first) {
                ptr = _s->first;
                _s->first = _s->first->next;
                free_urecord(ptr);
        }
-       
+       rec_lock_destroy(&_s->rlock);
+
        _s->n = 0;
        _s->last = 0;
     _s->d = 0;

diff --git a/modules/usrloc/hslot.h b/modules/usrloc/hslot.h
index ec1ddb0..a43b459 100644 (file)
--- a/modules/usrloc/hslot.h
+++ b/modules/usrloc/hslot.h
@@ -13,8 +13,8 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License 
- * along with this program; if not, write to the Free Software 
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
@@ -43,17 +43,13 @@ typedef struct hslot {
        struct urecord* first;  /*!< First element in the list */
        struct urecord* last;   /*!< Last element in the list */
        struct udomain* d;      /*!< Domain we belong to */
-#ifdef GEN_LOCK_T_PREFERED
-       gen_lock_t *lock;       /*!< Lock for hash entry - fastlock */
-#else
-       int lockidx;            /*!< Lock index for hash entry - the rest*/
-#endif
+       rec_lock_t rlock;       /*!< Recursive lock for hash entry */
 } hslot_t;
 
 /*! \brief
  * Initialize slot structure
  */
-void init_slot(struct udomain* _d, hslot_t* _s, int n);
+int init_slot(struct udomain* _d, hslot_t* _s, int n);
 
 
 /*! \brief
@@ -73,23 +69,4 @@ void slot_add(hslot_t* _s, struct urecord* _r);
  */
 void slot_rem(hslot_t* _s, struct urecord* _r);
 
-
-/*!
- * \brief Initialize locks for the hash table
- * \return 0 on success, -1 on failure
- */
-int ul_init_locks(void);
-
-
-/*!
- * \brief Destroy all locks on the list
- */
-void ul_unlock_locks(void);
-void ul_destroy_locks(void);
-
-#ifndef GEN_LOCK_T_PREFERED
-void ul_lock_idx(int idx);
-void ul_release_idx(int idx);
-#endif
-
 #endif /* HSLOT_H */

diff --git a/modules/usrloc/udomain.c b/modules/usrloc/udomain.c
index 58fc9a3..5042179 100644 (file)
--- a/modules/usrloc/udomain.c
+++ b/modules/usrloc/udomain.c
@@ -35,6 +35,7 @@
 #include "../../socket_info.h"
 #include "../../ut.h"
 #include "../../hashes.h"
+#include "../../sr_module.h"
 #include "ul_mod.h"            /* usrloc module parameters */
 #include "usrloc.h"
 #include "utime.h"
@@ -89,7 +90,7 @@ int new_udomain(str* _n, int _s, udomain_t** _d)
                goto error0;
        }
        memset(*_d, 0, sizeof(udomain_t));
-       
+
        (*_d)->table = (hslot_t*)shm_malloc(sizeof(hslot_t) * _s);
        if (!(*_d)->table) {
                LM_ERR("no memory left 2\n");
@@ -97,9 +98,12 @@ int new_udomain(str* _n, int _s, udomain_t** _d)
        }
 
        (*_d)->name = _n;
-       
+
        for(i = 0; i < _s; i++) {
-               init_slot(*_d, &((*_d)->table[i]), i);
+               if(init_slot(*_d, &((*_d)->table[i]), i)<0) {
+                       LM_ERR("failed to init hash table slot %d\n", i);
+                       goto error2;
+               }
        }
 
        (*_d)->size = _s;
@@ -145,9 +149,7 @@ void free_udomain(udomain_t* _d)
        
        if (_d->table) {
                for(i = 0; i < _d->size; i++) {
-                       lock_ulslot(_d, i);
                        deinit_slot(_d->table + i);
-                       unlock_ulslot(_d, i);
                }
                shm_free(_d->table);
        }
@@ -1005,7 +1007,7 @@ void mem_timer_udomain(udomain_t* _d, int istart, int istep)
 
        for(i=istart; i<_d->size; i+=istep)
        {
-               lock_ulslot(_d, i);
+               if(likely(destroy_modules_phase()==0)) lock_ulslot(_d, i);
 
                ptr = _d->table[i].first;
 
@@ -1020,7 +1022,7 @@ void mem_timer_udomain(udomain_t* _d, int istart, int istep)
                                ptr = ptr->next;
                        }
                }
-               unlock_ulslot(_d, i);
+               if(likely(destroy_modules_phase()==0)) unlock_ulslot(_d, i);
        }
 }
 
@@ -1037,11 +1039,7 @@ void lock_udomain(udomain_t* _d, str* _aor)
        {
                sl = ul_get_aorhash(_aor) & (_d->size - 1);
 
-#ifdef GEN_LOCK_T_PREFERED
-               lock_get(_d->table[sl].lock);
-#else
-               ul_lock_idx(_d->table[sl].lockidx);
-#endif
+               rec_lock_get(&_d->table[sl].rlock);
        }
 }
 
@@ -1057,11 +1055,7 @@ void unlock_udomain(udomain_t* _d, str* _aor)
        if (db_mode!=DB_ONLY)
        {
                sl = ul_get_aorhash(_aor) & (_d->size - 1);
-#ifdef GEN_LOCK_T_PREFERED
-               lock_release(_d->table[sl].lock);
-#else
-               ul_release_idx(_d->table[sl].lockidx);
-#endif
+               rec_lock_release(&_d->table[sl].rlock);
        }
 }
 
@@ -1073,11 +1067,7 @@ void unlock_udomain(udomain_t* _d, str* _aor)
 void lock_ulslot(udomain_t* _d, int i)
 {
        if (db_mode!=DB_ONLY)
-#ifdef GEN_LOCK_T_PREFERED
-               lock_get(_d->table[i].lock);
-#else
-               ul_lock_idx(_d->table[i].lockidx);
-#endif
+               rec_lock_get(&_d->table[i].rlock);
 }
 
 
@@ -1089,11 +1079,7 @@ void lock_ulslot(udomain_t* _d, int i)
 void unlock_ulslot(udomain_t* _d, int i)
 {
        if (db_mode!=DB_ONLY)
-#ifdef GEN_LOCK_T_PREFERED
-               lock_release(_d->table[i].lock);
-#else
-               ul_release_idx(_d->table[i].lockidx);
-#endif
+               rec_lock_release(&_d->table[i].rlock);
 }
 
 

diff --git a/modules/usrloc/ul_mod.c b/modules/usrloc/ul_mod.c
index 0e1a452..c111ea7 100644 (file)
--- a/modules/usrloc/ul_mod.c
+++ b/modules/usrloc/ul_mod.c
@@ -105,7 +105,6 @@ static int ul_preload_index = 0;
 static int ul_preload_param(modparam_t type, void* val);
 
 extern int bind_usrloc(usrloc_api_t* api);
-extern int ul_locks_no;
 int ul_db_update_as_insert = 0;
 int ul_timer_procs = 0;
 int ul_db_check_update = 0;
@@ -314,7 +313,6 @@ static int mod_init(void)
                ul_hash_size = 512;
        else
                ul_hash_size = 1<<ul_hash_size;
-       ul_locks_no = ul_hash_size;
 
        /* check matching mode */
        switch (matching_mode) {
@@ -327,12 +325,6 @@ static int mod_init(void)
                        LM_ERR("invalid matching mode %d\n", matching_mode);
        }
 
-       if(ul_init_locks()!=0)
-       {
-               LM_ERR("locks array initialization failed\n");
-               return -1;
-       }
-
        /* Register cache timer */
        if(ul_timer_procs<=0)
        {
@@ -493,7 +485,6 @@ static void destroy(void)
 {
        /* we need to sync DB in order to flush the cache */
        if (ul_dbh) {
-               ul_unlock_locks();
                if (synchronize_all_udomains(0, 1) != 0) {
                        LM_ERR("flushing cache failed\n");
                }
@@ -501,7 +492,6 @@ static void destroy(void)
        }
 
        free_all_udomains();
-       ul_destroy_locks();
 
        /* free callbacks list */
        destroy_ulcb_list();