Skip to content

Commit

Permalink
Bug 1363932 - reduce locking overhead in sftk_searchObjectList r=fran…
Browse files Browse the repository at this point in the history
…ziskus

Summary:
Before this patch, sftk_searchObjectList would acquire and release the
SFTKSlot's objectLock once per bucket in the SFTKSlot's sessObjHashTable. This
patch reduces the locking overhead by acquiring the lock once and then iterating
over the entire table.

This patch also removes the unused PRBool tokenOnly in NSC_FindObjectsInit (the
only caller of sftk_searchObjectList). (Changeset b8f289456399 removed the code
that modified tokenOnly. Unfortunately there doesn't appear to be a bug
associated with that changeset, so we can't easily determine if that was
intended. In any case, it's been this way for a decade, so if anything was
broken by this, no one has noticed yet.)

Reviewers: franziskus

Reviewed By: franziskus

Differential Revision: https://nss-review.dev.mozaws.net/D316

--HG--
extra : amend_source : b9454fd1fcd5fc587c41e7c39eb81d031730168e
  • Loading branch information
mozkeeler committed May 11, 2017
1 parent 135560d commit 477cca5
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 13 deletions.
14 changes: 5 additions & 9 deletions lib/softoken/pkcs11.c
Expand Up @@ -4763,7 +4763,7 @@ sftk_pruneSearch(CK_ATTRIBUTE *pTemplate, CK_ULONG ulCount,
static CK_RV
sftk_searchTokenList(SFTKSlot *slot, SFTKSearchResults *search,
CK_ATTRIBUTE *pTemplate, CK_ULONG ulCount,
PRBool *tokenOnly, PRBool isLoggedIn)
PRBool isLoggedIn)
{
CK_RV crv = CKR_OK;
CK_RV crv2;
Expand Down Expand Up @@ -4798,7 +4798,6 @@ NSC_FindObjectsInit(CK_SESSION_HANDLE hSession,
SFTKSearchResults *search = NULL, *freeSearch = NULL;
SFTKSession *session = NULL;
SFTKSlot *slot = sftk_SlotFromSessionHandle(hSession);
PRBool tokenOnly = PR_FALSE;
CK_RV crv = CKR_OK;
PRBool isLoggedIn;

Expand Down Expand Up @@ -4829,18 +4828,15 @@ NSC_FindObjectsInit(CK_SESSION_HANDLE hSession,
search->array_size = NSC_SEARCH_BLOCK_SIZE;
isLoggedIn = (PRBool)((!slot->needLogin) || slot->isLoggedIn);

crv = sftk_searchTokenList(slot, search, pTemplate, ulCount, &tokenOnly,
isLoggedIn);
crv = sftk_searchTokenList(slot, search, pTemplate, ulCount, isLoggedIn);
if (crv != CKR_OK) {
goto loser;
}

/* build list of found objects in the session */
if (!tokenOnly) {
crv = sftk_searchObjectList(search, slot->sessObjHashTable,
slot->sessObjHashSize, slot->objectLock,
pTemplate, ulCount, isLoggedIn);
}
crv = sftk_searchObjectList(search, slot->sessObjHashTable,
slot->sessObjHashSize, slot->objectLock,
pTemplate, ulCount, isLoggedIn);
if (crv != CKR_OK) {
goto loser;
}
Expand Down
6 changes: 2 additions & 4 deletions lib/softoken/pkcs11u.c
Expand Up @@ -1649,10 +1649,8 @@ sftk_searchObjectList(SFTKSearchResults *search, SFTKObject **head,
SFTKObject *object;
CK_RV crv = CKR_OK;

PZ_Lock(lock);
for (i = 0; i < size; i++) {
/* We need to hold the lock to copy a consistant version of
* the linked list. */
PZ_Lock(lock);
for (object = head[i]; object != NULL; object = object->next) {
if (sftk_objectMatch(object, theTemplate, count)) {
/* don't return objects that aren't yet visible */
Expand All @@ -1661,8 +1659,8 @@ sftk_searchObjectList(SFTKSearchResults *search, SFTKObject **head,
sftk_addHandle(search, object->handle);
}
}
PZ_Unlock(lock);
}
PZ_Unlock(lock);
return crv;
}

Expand Down

0 comments on commit 477cca5

Please sign in to comment.