From 52e0e125322fb0cc5c608be4cd43b90a702d88e2 Mon Sep 17 00:00:00 2001 From: Malahal Naineni Date: Fri, 21 Apr 2017 16:11:34 +0530 Subject: [PATCH] Fix nlm state refcount going up from zero Once a refcount goes to zero, we should not be using it any more and let the thread that made it zero free up the entry. If zero refcount entry is found in the hash table, don't use it, remove it from the hash table and insert a new nlm state. Also, under some cases, we do delete non zero refcnt nlm state objects from the hash table as well. This change should avoid logging spurious messages from dec_nlm_state_ref() for not finding the nlm state in the hash table and accidentally deleting someone else's nlm state. Change-Id: I3917327f2fdfe8df0e5d9d2a64786ba4c1eb4cd4 Signed-off-by: Malahal Naineni --- src/SAL/nlm_state.c | 73 ++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/src/SAL/nlm_state.c b/src/SAL/nlm_state.c index 4b24ff8a17..30a0e4f9a7 100644 --- a/src/SAL/nlm_state.c +++ b/src/SAL/nlm_state.c @@ -264,7 +264,6 @@ void dec_nlm_state_ref(state_t *state) hash_error_t rc; struct gsh_buffdesc buffkey; struct gsh_buffdesc old_value; - struct gsh_buffdesc old_key; int32_t refcount; struct fsal_obj_handle *obj; @@ -294,10 +293,22 @@ void dec_nlm_state_ref(state_t *state) rc = hashtable_getlatch(ht_nlm_states, &buffkey, &old_value, true, &latch); - if (rc != HASHTABLE_SUCCESS) { - if (rc == HASHTABLE_ERROR_NO_SUCH_KEY) - hashtable_releaselatched(ht_nlm_states, &latch); + /* Another thread that needs this entry might have deleted this + * nlm state to insert its own nlm state. So expect not to find + * this nlm state or find someone else's nlm state! + */ + switch (rc) { + case HASHTABLE_SUCCESS: + if (old_value.addr == state) { /* our own state */ + hashtable_deletelatched(ht_nlm_states, &buffkey, + &latch, NULL, NULL); + } + break; + case HASHTABLE_ERROR_NO_SUCH_KEY: + break; + + default: if (!str_valid) display_nlm_state(&dspbuf, state); @@ -307,23 +318,6 @@ void dec_nlm_state_ref(state_t *state) return; } - refcount = atomic_fetch_int32_t(&state->state_refcount); - - if (refcount > 0) { - if (str_valid) - LogDebug(COMPONENT_STATE, - "Did not release refcount now=%"PRId32" {%s}", - refcount, str); - - hashtable_releaselatched(ht_nlm_states, &latch); - - return; - } - - /* use the key to delete the entry */ - hashtable_deletelatched(ht_nlm_states, &buffkey, &latch, &old_key, - &old_value); - /* Release the latch */ hashtable_releaselatched(ht_nlm_states, &latch); @@ -379,8 +373,8 @@ int get_nlm_state(enum state_type state_type, struct display_buffer dspbuf = {sizeof(str), str, str}; struct hash_latch latch; hash_error_t rc; - struct gsh_buffdesc buffkey, old_key; - struct gsh_buffdesc buffval, old_value; + struct gsh_buffdesc buffkey; + struct gsh_buffdesc buffval; *pstate = NULL; memset(&key, 0, sizeof(key)); @@ -402,26 +396,26 @@ int get_nlm_state(enum state_type state_type, rc = hashtable_getlatch(ht_nlm_states, &buffkey, &buffval, true, &latch); - /* If we found it, return it */ - if (rc == HASHTABLE_SUCCESS) { + switch (rc) { + case HASHTABLE_SUCCESS: state = buffval.addr; - if (nsm_state_applies && state->state_seqid != nsm_state) { - /* We are getting new locks before the old ones are - * gone. We need to unhash this state_t and create a - * new one. + if (nsm_state_applies && + (state->state_seqid != nsm_state || + atomic_fetch_int32_t(&state->state_refcount) == 0)) { + /* We are getting new locks before the old ones + * are gone or the state is in the process of + * getting deleted. We need to unhash this + * state_t and create a new one. * * Keep the latch after the delete to proceed with * the new insert. */ /* use the key to delete the entry */ - hashtable_deletelatched(ht_nlm_states, - &buffkey, - &latch, - &old_key, - &old_value); - goto new_state; + hashtable_deletelatched(ht_nlm_states, &buffkey, + &latch, NULL, NULL); + break; } /* Return the found NLM State */ @@ -440,10 +434,11 @@ int get_nlm_state(enum state_type state_type, *pstate = state; return 0; - } - /* An error occurred, return NULL */ - if (rc != HASHTABLE_ERROR_NO_SUCH_KEY) { + case HASHTABLE_ERROR_NO_SUCH_KEY: + break; + + default: /* An error occurred, return NULL */ display_nlm_state(&dspbuf, &key); LogCrit(COMPONENT_STATE, "Error %s, could not find {%s}", @@ -454,8 +449,6 @@ int get_nlm_state(enum state_type state_type, return NLM4_DENIED_NOLOCKS; } - new_state: - /* If the nsm state doesn't apply, we don't want to create a new * state_t if one didn't exist already. */