Browse Source

Ticket 509 - lock-free access to be->be_suffixlock

Bug Description:  Remove locking around the be_suffixcount

Fix Description:  Use atomic counter for the suffix count, and remove
                  the lock.  The "count" is all we need to safely transverse
                  the array.

https://fedorahosted.org/389/ticket/509

Reviewed by: Ludwig & richm(Thanks!)
Mark Reynolds 13 years ago
parent
commit
06a26b6cdb
3 changed files with 34 additions and 32 deletions
  1. 18 16
      ldap/servers/slapd/backend.c
  2. 15 15
      ldap/servers/slapd/backend_manager.c
  3. 1 1
      ldap/servers/slapd/slap.h

+ 18 - 16
ldap/servers/slapd/backend.c

@@ -49,8 +49,8 @@ be_init( Slapi_Backend *be, const char *type, const char *name, int isprivate, i
 {
     slapdFrontendConfig_t *fecfg;
     be->be_suffix = NULL;
-    be->be_suffixlock= PR_NewLock();
-    be->be_suffixcount= 0;
+    be->be_suffixlock = PR_NewLock();
+    be->be_suffixcounter = slapi_counter_new();
     /* e.g. dn: cn=config,cn=NetscapeRoot,cn=ldbm database,cn=plugins,cn=config */
     be->be_basedn = slapi_create_dn_string("cn=%s,cn=%s,cn=plugins,cn=config",
                                            name, type);
@@ -116,13 +116,13 @@ void
 be_done(Slapi_Backend *be)
 {
     int i;
+    int count = slapi_counter_get_value(be->be_suffixcounter);
 
-    for(i=0;i<be->be_suffixcount;i++)
+    for(i=0; i < count; i++)
     {
         slapi_sdn_free(&be->be_suffix[i]);
     }
     slapi_ch_free((void**)&be->be_suffix);
-    PR_DestroyLock(be->be_suffixlock);
     slapi_ch_free((void **)&be->be_basedn);
     slapi_ch_free((void **)&be->be_configdn);
     slapi_ch_free((void **)&be->be_monitordn);
@@ -133,6 +133,8 @@ be_done(Slapi_Backend *be)
     if (!config_get_entryusn_global()) {
         slapi_counter_destroy(&be->be_usn_counter);
     }
+    slapi_counter_destroy(&be->be_suffixcounter);
+    PR_DestroyLock(be->be_suffixlock);
     PR_DestroyLock(be->be_state_lock);
     if (be->be_lock != NULL)
     {
@@ -170,9 +172,9 @@ slapi_be_issuffix( const Slapi_Backend *be, const Slapi_DN *suffix )
 	/* this backend is no longer valid */
 	if (be->be_state != BE_STATE_DELETED)
 	{
-    	int	i;
-        PR_Lock(be->be_suffixlock);
-    	for ( i = 0; be->be_suffix != NULL && i<be->be_suffixcount; i++ )
+    	int	i, count;
+    	count = slapi_counter_get_value(be->be_suffixcounter);
+    	for ( i = 0; be->be_suffix != NULL && i < count; i++ )
 		{
     		if ( slapi_sdn_compare( be->be_suffix[i], suffix ) == 0)
 		    {
@@ -180,7 +182,6 @@ slapi_be_issuffix( const Slapi_Backend *be, const Slapi_DN *suffix )
 				break;
     		}
     	}
-        PR_Unlock(be->be_suffixlock);
 	}
 	return r;
 }
@@ -196,18 +197,21 @@ be_addsuffix(Slapi_Backend *be,const Slapi_DN *suffix)
 {
 	if (be->be_state != BE_STATE_DELETED)
 	{
-        PR_Lock(be->be_suffixlock);
+		int count;
+
+		PR_Lock(be->be_suffixlock);
+		count = slapi_counter_get_value(be->be_suffixcounter);
 		if(be->be_suffix==NULL)
 		{
 		    be->be_suffix= (Slapi_DN **)slapi_ch_malloc(sizeof(Slapi_DN *));
 		}
 		else
 		{
-		    be->be_suffix= (Slapi_DN **)slapi_ch_realloc((char*)be->be_suffix,(be->be_suffixcount+1)*sizeof(Slapi_DN *));
+		    be->be_suffix= (Slapi_DN **)slapi_ch_realloc((char*)be->be_suffix,(count+1)*sizeof(Slapi_DN *));
 		}
-		be->be_suffix[be->be_suffixcount]= slapi_sdn_dup(suffix);
-        be->be_suffixcount++;
-        PR_Unlock(be->be_suffixlock);
+		be->be_suffix[count]= slapi_sdn_dup(suffix);
+		slapi_counter_increment(be->be_suffixcounter);
+		PR_Unlock(be->be_suffixlock);
 	}
 }
 
@@ -231,11 +235,9 @@ slapi_be_getsuffix(Slapi_Backend *be,int n)
 		return NULL;
 
     if(be->be_state != BE_STATE_DELETED) {
-        PR_Lock(be->be_suffixlock);
-        if (be->be_suffix !=NULL && n<be->be_suffixcount) {
+        if (be->be_suffix !=NULL && n < slapi_counter_get_value(be->be_suffixcounter)) {
             sdn =  be->be_suffix[n];
         }
-        PR_Unlock(be->be_suffixlock);
     }
     return sdn;
 }

+ 15 - 15
ldap/servers/slapd/backend_manager.c

@@ -461,11 +461,12 @@ int
 slapi_lookup_instance_name_by_suffix(char *suffix,
 							char ***suffixes, char ***instances, int isexact)
 {
-    Slapi_Backend *be = NULL;
-    char *cookie = NULL;
+	Slapi_Backend *be = NULL;
+	char *cookie = NULL;
 	const char *thisdn;
 	int thisdnlen;
 	int suffixlen;
+	int count;
 	int i;
 	int rval = -1;
 
@@ -476,17 +477,17 @@ slapi_lookup_instance_name_by_suffix(char *suffix,
 
 	rval = 0;
 	suffixlen = strlen(suffix);
-    cookie = NULL;
-    be = slapi_get_first_backend (&cookie);
-    while (be) {
-       	if (NULL == be->be_suffix) {
-       		be = (backend *)slapi_get_next_backend (cookie);
+	cookie = NULL;
+	be = slapi_get_first_backend (&cookie);
+	while (be) {
+		if (NULL == be->be_suffix) {
+			be = (backend *)slapi_get_next_backend (cookie);
 			continue;
 		}
-        PR_Lock(be->be_suffixlock);
-    	for (i = 0; be->be_suffix && i < be->be_suffixcount; i++) {
-    		thisdn = slapi_sdn_get_ndn(be->be_suffix[i]);
-    		thisdnlen = slapi_sdn_get_ndn_len(be->be_suffix[i]);
+		count = slapi_counter_get_value(be->be_suffixcounter);
+		for (i = 0; be->be_suffix && i < count; i++) {
+			thisdn = slapi_sdn_get_ndn(be->be_suffix[i]);
+			thisdnlen = slapi_sdn_get_ndn_len(be->be_suffix[i]);
 			if (isexact?suffixlen!=thisdnlen:suffixlen>thisdnlen)
 				continue;
 			if (isexact?(!slapi_UTF8CASECMP(suffix, (char *)thisdn)):
@@ -497,10 +498,9 @@ slapi_lookup_instance_name_by_suffix(char *suffix,
 					charray_add(suffixes, slapi_ch_strdup(thisdn));
 			}
 		}
-        PR_Unlock(be->be_suffixlock);
-       	be = (backend *)slapi_get_next_backend (cookie);
-    }
-    slapi_ch_free((void **)&cookie);
+		be = (backend *)slapi_get_next_backend (cookie);
+	}
+	slapi_ch_free((void **)&cookie);
 	
 	return rval;
 }

+ 1 - 1
ldap/servers/slapd/slap.h

@@ -1165,7 +1165,7 @@ struct slapdplugin {
 typedef struct backend {
 	Slapi_DN **be_suffix;    /* the DN suffixes of data in this backend */
     PRLock *be_suffixlock;
-    int be_suffixcount;
+    Slapi_Counter *be_suffixcounter;
     char *be_basedn;     /* The base dn for the config & monitor dns */
 	char *be_configdn;   /* The config dn for this backend          */
 	char *be_monitordn;  /* The monitor dn for this backend          */