ldap/servers/slapd/back-ldbm/back-ldbm.h | 22 +++++++++++----------- ldap/servers/slapd/back-ldbm/backentry.c | 2 +- ldap/servers/slapd/back-ldbm/cache.c | 19 ++++++++++++++----- ldap/servers/slapd/back-ldbm/id2entry.c | 1 - ldap/servers/slapd/back-ldbm/ldbm_delete.c | 6 ++---- ldap/servers/slapd/back-ldbm/ldbm_modify.c | 6 ++---- ldap/servers/slapd/modify.c | 4 ++++ 7 files changed, 34 insertions(+), 26 deletions(-)
New commits: commit 9a9f93c0b47a1755fd1bb637f35a730108119779 Author: Noriko Hosoi nhosoi@redhat.com Date: Fri Oct 28 11:37:53 2011 -0700
Bug 745259 - Incorrect entryUSN index under high load in replicated environment
https://bugzilla.redhat.com/show_bug.cgi?id=745259
Description: . Changed the backend entry lock from PRLock to PRMonitor to allow re-entrant locking. In ldbm_back_delete and ldbm_ back_modify, backend entry lock was released before calling be_preop plugins and re-acquired just after that to avoid the deadlock by the be_preop plugins called from the same thread. The short no-lock window was the cause of the entry- usn corruption. By replacing PRLock with the re-entrant PRMonitor, we could eliminate the no-lock window. . Cleaned up compiler warnings.
diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h index f5668d2..db0c833 100644 --- a/ldap/servers/slapd/back-ldbm/back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h @@ -350,19 +350,19 @@ struct backcommon {
/* From ep_type through ep_size MUST be identical to backcommon */ struct backentry { - int ep_type; /* to distinguish backdn from backentry */ - struct backcommon *ep_lrunext; /* for the cache */ - struct backcommon *ep_lruprev; /* for the cache */ - ID ep_id; /* entry id */ - char ep_state; /* state in the cache */ - int ep_refcnt; /* entry reference cnt */ + int ep_type; /* to distinguish backdn from backentry */ + struct backcommon *ep_lrunext; /* for the cache */ + struct backcommon *ep_lruprev; /* for the cache */ + ID ep_id; /* entry id */ + char ep_state; /* state in the cache */ + int ep_refcnt; /* entry reference cnt */ size_t ep_size; /* for cache tracking */ - Slapi_Entry *ep_entry; /* real entry */ + Slapi_Entry *ep_entry; /* real entry */ Slapi_Entry *ep_vlventry; - void * ep_dn_link; /* linkage for the 3 hash */ - void * ep_id_link; /* tables used for */ - void * ep_uuid_link; /* looking up entries */ - PRLock *ep_mutexp; /* protection for mods */ + void * ep_dn_link; /* linkage for the 3 hash */ + void * ep_id_link; /* tables used for */ + void * ep_uuid_link; /* looking up entries */ + PRMonitor *ep_mutexp; /* protection for mods; make it reentrant */ };
/* From ep_type through ep_size MUST be identical to backcommon */ diff --git a/ldap/servers/slapd/back-ldbm/backentry.c b/ldap/servers/slapd/back-ldbm/backentry.c index c26f552..9fe6fb6 100644 --- a/ldap/servers/slapd/back-ldbm/backentry.c +++ b/ldap/servers/slapd/back-ldbm/backentry.c @@ -56,7 +56,7 @@ backentry_free( struct backentry **bep ) slapi_entry_free( ep->ep_entry ); } if ( ep->ep_mutexp != NULL ) { - PR_DestroyLock( ep->ep_mutexp ); + PR_DestroyMonitor( ep->ep_mutexp ); } slapi_ch_free( (void**)&ep ); *bep = NULL; diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 3d09bb3..e2f1768 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -1437,19 +1437,28 @@ int cache_lock_entry(struct cache *cache, struct backentry *e) if (! e->ep_mutexp) { /* make sure only one thread does this */ PR_Lock(cache->c_emutexalloc_mutex); - if (! e->ep_mutexp) - e->ep_mutexp = PR_NewLock(); + if (! e->ep_mutexp) { + e->ep_mutexp = PR_NewMonitor(); + if (!e->ep_mutexp) { + LOG("<= cache_lock_entry (DELETED)\n", 0, 0, 0); + LDAPDebug1Arg(LDAP_DEBUG_ANY, + "cache_lock_entry: failed to create a lock for %s\n", + backentry_get_ndn(e)); + LOG("<= cache_lock_entry (FAILED)\n", 0, 0, 0); + return 1; + } + } PR_Unlock(cache->c_emutexalloc_mutex); }
/* wait on entry lock (done w/o holding the cache lock) */ - PR_Lock(e->ep_mutexp); + PR_EnterMonitor(e->ep_mutexp);
/* make sure entry hasn't been deleted now */ PR_Lock(cache->c_mutex); if (e->ep_state & (ENTRY_STATE_DELETED|ENTRY_STATE_NOTINCACHE)) { PR_Unlock(cache->c_mutex); - PR_Unlock(e->ep_mutexp); + PR_ExitMonitor(e->ep_mutexp); LOG("<= cache_lock_entry (DELETED)\n", 0, 0, 0); return 1; } @@ -1463,7 +1472,7 @@ int cache_lock_entry(struct cache *cache, struct backentry *e) void cache_unlock_entry(struct cache *cache, struct backentry *e) { LOG("=> cache_unlock_entry\n", 0, 0, 0); - PR_Unlock(e->ep_mutexp); + PR_ExitMonitor(e->ep_mutexp); }
/* DN cache */ diff --git a/ldap/servers/slapd/back-ldbm/id2entry.c b/ldap/servers/slapd/back-ldbm/id2entry.c index bd51a7f..91a4ff0 100644 --- a/ldap/servers/slapd/back-ldbm/id2entry.c +++ b/ldap/servers/slapd/back-ldbm/id2entry.c @@ -95,7 +95,6 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt memset(&data, 0, sizeof(data)); if (entryrdn_get_switch()) { - Slapi_Attr *eattr = NULL; struct backdn *oldbdn = NULL; Slapi_DN *sdn = slapi_sdn_dup(slapi_entry_get_sdn_const(entry_to_use)); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index c1f1142..46bfcc3 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -210,11 +210,9 @@ ldbm_back_delete( Slapi_PBlock *pb ) goto error_return; } slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); - /* have to unlock the entry here, in case the bepreop attempts - to modify the same entry == deadlock */ - cache_unlock_entry( &inst->inst_cache, e ); + rc = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_DELETE_FN); - cache_lock_entry( &inst->inst_cache, e ); + if (rc == -1) { /* diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index 1a85e14..ba6527f 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -307,11 +307,9 @@ ldbm_back_modify( Slapi_PBlock *pb ) /* ec is the entry that our bepreop should get to mess with */ slapi_pblock_set( pb, SLAPI_MODIFY_EXISTING_ENTRY, ec->ep_entry ); slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); - /* have to unlock the entry here, in case the bepreop attempts - to modify the same entry == deadlock */ - cache_unlock_entry( &inst->inst_cache, e ); + plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_MODIFY_FN); - cache_lock_entry( &inst->inst_cache, e ); + slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code); /* The Plugin may have messed about with some of the PBlock parameters... ie. mods */ slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 45e55b2..a0af006 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -75,7 +75,9 @@ /* Forward declarations */ static int modify_internal_pb (Slapi_PBlock *pb); static void op_shared_modify (Slapi_PBlock *pb, int pw_change, char *old_pw); +#if 0 /* not used */ static void remove_mod (Slapi_Mods *smods, const char *type, Slapi_Mods *smod_unhashed); +#endif static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old_pw, Slapi_Mods *smods); static int hash_rootpw (LDAPMod **mods);
@@ -976,6 +978,7 @@ free_and_return: } }
+#if 0 /* not used */ static void remove_mod (Slapi_Mods *smods, const char *type, Slapi_Mods *smod_unhashed) { LDAPMod *mod; @@ -991,6 +994,7 @@ static void remove_mod (Slapi_Mods *smods, const char *type, Slapi_Mods *smod_un } } } +#endif
static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old_pw, Slapi_Mods *smods) {
389-commits@lists.fedoraproject.org
-
Noriko Hosoi