git: 724072ab1d58 - main - nfscl: Use hash lists to improve expected search performance for opens
Rick Macklem
rmacklem at FreeBSD.org
Tue May 25 21:25:40 UTC 2021
The branch main has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=724072ab1d588677a83a5a5011b5ad9ff5d56538
commit 724072ab1d588677a83a5a5011b5ad9ff5d56538
Author: Rick Macklem <rmacklem at FreeBSD.org>
AuthorDate: 2021-05-25 21:19:29 +0000
Commit: Rick Macklem <rmacklem at FreeBSD.org>
CommitDate: 2021-05-25 21:19:29 +0000
nfscl: Use hash lists to improve expected search performance for opens
A problem was reported via email, where a large (130000+) accumulation
of NFSv4 opens on an NFSv4 mount caused significant lock contention
on the mutex used to protect the client mount's open/lock state.
Although the root cause for the accumulation of opens was not
resolved, it is obvious that the NFSv4 client is not designed to
handle 100000+ opens efficiently. When searching for an open,
usually for a match by file handle, a linear search of all opens
is done.
Commit 3f7e14ad9345 added a hash table of lists hashed on file handle
for the opens. This patch uses the hash lists for searching for
a matching open based of file handle instead of an exhaustive
linear search of all opens.
This change appears to be performance neutral for a small number
of opens, but should improve expected performance for a large
number of opens. This patch also moves any found match to the front
of the hash list, to try and maintain the hash lists in recently
used ordering (least recently used at the end of the list).
This commit should not affect the high level semantics of open
handling.
MFC after: 2 weeks
---
sys/fs/nfsclient/nfs_clstate.c | 89 ++++++++++++++++++++++++++----------------
1 file changed, 56 insertions(+), 33 deletions(-)
diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c
index a8eace2ffd0b..63c70ebcfdf5 100644
--- a/sys/fs/nfsclient/nfs_clstate.c
+++ b/sys/fs/nfsclient/nfs_clstate.c
@@ -100,8 +100,9 @@ int nfscl_layouthighwater = NFSCLLAYOUTHIGHWATER;
static int nfscl_delegcnt = 0;
static int nfscl_layoutcnt = 0;
-static int nfscl_getopen(struct nfsclownerhead *, u_int8_t *, int, u_int8_t *,
- u_int8_t *, u_int32_t, struct nfscllockowner **, struct nfsclopen **);
+static int nfscl_getopen(struct nfsclownerhead *, struct nfsclopenhash *,
+ u_int8_t *, int, u_int8_t *, u_int8_t *, u_int32_t,
+ struct nfscllockowner **, struct nfsclopen **);
static bool nfscl_checkown(struct nfsclowner *, struct nfsclopen *, uint8_t *,
uint8_t *, struct nfscllockowner **, struct nfsclopen **,
struct nfsclopen **);
@@ -509,8 +510,8 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
void **lckpp)
{
struct nfsclclient *clp;
- struct nfsclowner *owp;
struct nfsclopen *op = NULL, *top;
+ struct nfsclopenhash *oph;
struct nfscllockowner *lp;
struct nfscldeleg *dp;
struct nfsnode *np;
@@ -587,8 +588,8 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
else
nfscl_filllockowner(p->td_proc, own, F_POSIX);
lp = NULL;
- error = nfscl_getopen(&clp->nfsc_owner, nfhp, fhlen, own, own,
- mode, &lp, &op);
+ error = nfscl_getopen(NULL, clp->nfsc_openhash, nfhp, fhlen,
+ own, own, mode, &lp, &op);
if (error == 0 && lp != NULL && fords == 0) {
/* Don't return a lock stateid for a DS. */
stateidp->seqid =
@@ -607,22 +608,22 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
/* If not found, just look for any OpenOwner that will work. */
top = NULL;
done = false;
- LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) {
- LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
- if (op->nfso_fhlen == fhlen &&
- !NFSBCMP(op->nfso_fh, nfhp, fhlen)) {
- if (top == NULL && (op->nfso_mode &
- NFSV4OPEN_ACCESSWRITE) != 0 &&
- (mode & NFSV4OPEN_ACCESSREAD) != 0)
- top = op;
- if ((mode & op->nfso_mode) == mode) {
- done = true;
- break;
- }
+ oph = NFSCLOPENHASH(clp, nfhp, fhlen);
+ LIST_FOREACH(op, oph, nfso_hash) {
+ if (op->nfso_fhlen == fhlen &&
+ !NFSBCMP(op->nfso_fh, nfhp, fhlen)) {
+ if (top == NULL && (op->nfso_mode &
+ NFSV4OPEN_ACCESSWRITE) != 0 &&
+ (mode & NFSV4OPEN_ACCESSREAD) != 0)
+ top = op;
+ if ((mode & op->nfso_mode) == mode) {
+ /* LRU order the hash list. */
+ LIST_REMOVE(op, nfso_hash);
+ LIST_INSERT_HEAD(oph, op, nfso_hash);
+ done = true;
+ break;
}
}
- if (done)
- break;
}
if (!done) {
NFSCL_DEBUG(2, "openmode top=%p\n", top);
@@ -655,14 +656,17 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
* Search for a matching file, mode and, optionally, lockowner.
*/
static int
-nfscl_getopen(struct nfsclownerhead *ohp, u_int8_t *nfhp, int fhlen,
- u_int8_t *openown, u_int8_t *lockown, u_int32_t mode,
- struct nfscllockowner **lpp, struct nfsclopen **opp)
+nfscl_getopen(struct nfsclownerhead *ohp, struct nfsclopenhash *ohashp,
+ u_int8_t *nfhp, int fhlen, u_int8_t *openown, u_int8_t *lockown,
+ u_int32_t mode, struct nfscllockowner **lpp, struct nfsclopen **opp)
{
struct nfsclowner *owp;
struct nfsclopen *op, *rop, *rop2;
+ struct nfsclopenhash *oph;
bool keep_looping;
+ KASSERT(ohp == NULL || ohashp == NULL, ("nfscl_getopen: "
+ "only one of ohp and ohashp can be set"));
if (lpp != NULL)
*lpp = NULL;
/*
@@ -679,19 +683,38 @@ nfscl_getopen(struct nfsclownerhead *ohp, u_int8_t *nfhp, int fhlen,
rop2 = NULL;
keep_looping = true;
/* Search the client list */
- LIST_FOREACH(owp, ohp, nfsow_list) {
- /* and look for the correct open */
- LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
+ if (ohashp == NULL) {
+ /* Search the local opens on the delegation. */
+ LIST_FOREACH(owp, ohp, nfsow_list) {
+ /* and look for the correct open */
+ LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
+ if (op->nfso_fhlen == fhlen &&
+ !NFSBCMP(op->nfso_fh, nfhp, fhlen)
+ && (op->nfso_mode & mode) == mode)
+ keep_looping = nfscl_checkown(owp, op, openown,
+ lockown, lpp, &rop, &rop2);
+ if (!keep_looping)
+ break;
+ }
+ if (!keep_looping)
+ break;
+ }
+ } else {
+ /* Search for matching opens on the hash list. */
+ oph = &ohashp[NFSCLOPENHASHFUNC(nfhp, fhlen)];
+ LIST_FOREACH(op, oph, nfso_hash) {
if (op->nfso_fhlen == fhlen &&
!NFSBCMP(op->nfso_fh, nfhp, fhlen)
&& (op->nfso_mode & mode) == mode)
- keep_looping = nfscl_checkown(owp, op, openown,
- lockown, lpp, &rop, &rop2);
- if (!keep_looping)
+ keep_looping = nfscl_checkown(op->nfso_own, op,
+ openown, lockown, lpp, &rop, &rop2);
+ if (!keep_looping) {
+ /* LRU order the hash list. */
+ LIST_REMOVE(op, nfso_hash);
+ LIST_INSERT_HEAD(oph, op, nfso_hash);
break;
+ }
}
- if (!keep_looping)
- break;
}
if (rop == NULL)
rop = rop2;
@@ -1090,10 +1113,10 @@ nfscl_getbytelock(vnode_t vp, u_int64_t off, u_int64_t len,
}
if (dp != NULL) {
/* Now, find an open and maybe a lockowner. */
- ret = nfscl_getopen(&dp->nfsdl_owner, np->n_fhp->nfh_fh,
+ ret = nfscl_getopen(&dp->nfsdl_owner, NULL, np->n_fhp->nfh_fh,
np->n_fhp->nfh_len, openownp, ownp, mode, NULL, &op);
if (ret)
- ret = nfscl_getopen(&clp->nfsc_owner,
+ ret = nfscl_getopen(NULL, clp->nfsc_openhash,
np->n_fhp->nfh_fh, np->n_fhp->nfh_len, openownp,
ownp, mode, NULL, &op);
if (!ret) {
@@ -1110,7 +1133,7 @@ nfscl_getbytelock(vnode_t vp, u_int64_t off, u_int64_t len,
/*
* Get the related Open and maybe lockowner.
*/
- error = nfscl_getopen(&clp->nfsc_owner,
+ error = nfscl_getopen(NULL, clp->nfsc_openhash,
np->n_fhp->nfh_fh, np->n_fhp->nfh_len, openownp,
ownp, mode, &lp, &op);
if (!error)
More information about the dev-commits-src-all
mailing list