Date: Sat, 12 Jun 2021 01:30:42 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 972883b9e06e - stable/13 - nfscl: Use hash lists to improve expected search performance for opens Message-ID: <202106120130.15C1Ugpi025576@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=972883b9e06e32df319fea46e365ff7be498bc15 commit 972883b9e06e32df319fea46e365ff7be498bc15 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-05-28 02:08:36 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-06-12 01:21:03 +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 commit should not affect the high level semantics of open handling. (cherry picked from commit 96b40b896772bbce5f93d62eaa640e1eb51b4a30) --- sys/fs/nfsclient/nfs_clstate.c | 165 +++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 90 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index 63c70ebcfdf5..5e4ac2ae9d4d 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -1229,7 +1229,6 @@ nfscl_relbytelock(vnode_t vp, u_int64_t off, u_int64_t len, struct nfscllockowner **lpp, int *dorpcp) { struct nfscllockowner *lp; - struct nfsclowner *owp; struct nfsclopen *op; struct nfscllock *nlop, *other_lop = NULL; struct nfscldeleg *dp; @@ -1291,24 +1290,21 @@ nfscl_relbytelock(vnode_t vp, u_int64_t off, u_int64_t len, */ lp = NULL; fnd = 0; - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { - LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { + LIST_FOREACH(op, NFSCLOPENHASH(clp, np->n_fhp->nfh_fh, + np->n_fhp->nfh_len), nfso_hash) { if (op->nfso_fhlen == np->n_fhp->nfh_len && !NFSBCMP(op->nfso_fh, np->n_fhp->nfh_fh, op->nfso_fhlen)) { - LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { - if (lp->nfsl_inprog == NULL && - !NFSBCMP(lp->nfsl_owner, own, - NFSV4CL_LOCKNAMELEN)) { - fnd = 1; - break; + LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { + if (lp->nfsl_inprog == NULL && + !NFSBCMP(lp->nfsl_owner, own, + NFSV4CL_LOCKNAMELEN)) { + fnd = 1; + break; + } } - } - if (fnd) - break; } - } - if (fnd) - break; + if (fnd) + break; } if (lp != NULL) { @@ -1338,7 +1334,6 @@ void nfscl_releasealllocks(struct nfsclclient *clp, vnode_t vp, NFSPROC_T *p, void *id, int flags) { - struct nfsclowner *owp; struct nfsclopen *op; struct nfscllockowner *lp; struct nfsnode *np; @@ -1347,20 +1342,19 @@ nfscl_releasealllocks(struct nfsclclient *clp, vnode_t vp, NFSPROC_T *p, np = VTONFS(vp); nfscl_filllockowner(id, own, flags); NFSLOCKCLSTATE(); - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { - LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { + LIST_FOREACH(op, NFSCLOPENHASH(clp, np->n_fhp->nfh_fh, + np->n_fhp->nfh_len), nfso_hash) { if (op->nfso_fhlen == np->n_fhp->nfh_len && !NFSBCMP(op->nfso_fh, np->n_fhp->nfh_fh, op->nfso_fhlen)) { - LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { - if (lp->nfsl_inprog == p && - !NFSBCMP(lp->nfsl_owner, own, - NFSV4CL_LOCKNAMELEN)) { - lp->nfsl_inprog = NULL; - nfscl_lockunlock(&lp->nfsl_rwlock); + LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { + if (lp->nfsl_inprog == p && + !NFSBCMP(lp->nfsl_owner, own, + NFSV4CL_LOCKNAMELEN)) { + lp->nfsl_inprog = NULL; + nfscl_lockunlock(&lp->nfsl_rwlock); + } } - } } - } } nfscl_clrelease(clp); NFSUNLOCKCLSTATE(); @@ -1376,7 +1370,6 @@ int nfscl_checkwritelocked(vnode_t vp, struct flock *fl, struct ucred *cred, NFSPROC_T *p, void *id, int flags) { - struct nfsclowner *owp; struct nfscllockowner *lp; struct nfsclopen *op; struct nfsclclient *clp; @@ -1445,30 +1438,29 @@ nfscl_checkwritelocked(vnode_t vp, struct flock *fl, /* * Now, check state against the server. */ - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { - LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { + LIST_FOREACH(op, NFSCLOPENHASH(clp, np->n_fhp->nfh_fh, + np->n_fhp->nfh_len), nfso_hash) { if (op->nfso_fhlen == np->n_fhp->nfh_len && !NFSBCMP(op->nfso_fh, np->n_fhp->nfh_fh, op->nfso_fhlen)) { - LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { - if (!NFSBCMP(lp->nfsl_owner, own, - NFSV4CL_LOCKNAMELEN)) - break; - } - if (lp != NULL) { - LIST_FOREACH(lop, &lp->nfsl_lock, nfslo_list) { - if (lop->nfslo_first >= end) - break; - if (lop->nfslo_end <= off) - continue; - if (lop->nfslo_type == F_WRLCK) { - nfscl_clrelease(clp); - NFSUNLOCKCLSTATE(); - return (1); - } + LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) { + if (!NFSBCMP(lp->nfsl_owner, own, + NFSV4CL_LOCKNAMELEN)) + break; + } + if (lp != NULL) { + LIST_FOREACH(lop, &lp->nfsl_lock, nfslo_list) { + if (lop->nfslo_first >= end) + break; + if (lop->nfslo_end <= off) + continue; + if (lop->nfslo_type == F_WRLCK) { + nfscl_clrelease(clp); + NFSUNLOCKCLSTATE(); + return (1); + } + } } - } } - } } nfscl_clrelease(clp); NFSUNLOCKCLSTATE(); @@ -3243,23 +3235,22 @@ nfscl_getclose(vnode_t vp, struct nfsclclient **clpp) } /* Now process the opens against the server. */ - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { - LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { - if (op->nfso_fhlen == nfhp->nfh_len && - !NFSBCMP(op->nfso_fh, nfhp->nfh_fh, - nfhp->nfh_len)) { - /* Found an open, decrement cnt if possible */ - if (notdecr && op->nfso_opencnt > 0) { - notdecr = 0; - op->nfso_opencnt--; - } - /* - * There are more opens, so just return. - */ - if (op->nfso_opencnt > 0) { - NFSUNLOCKCLSTATE(); - return (0); - } + LIST_FOREACH(op, NFSCLOPENHASH(clp, nfhp->nfh_fh, nfhp->nfh_len), + nfso_hash) { + if (op->nfso_fhlen == nfhp->nfh_len && + !NFSBCMP(op->nfso_fh, nfhp->nfh_fh, + nfhp->nfh_len)) { + /* Found an open, decrement cnt if possible */ + if (notdecr && op->nfso_opencnt > 0) { + notdecr = 0; + op->nfso_opencnt--; + } + /* + * There are more opens, so just return. + */ + if (op->nfso_opencnt > 0) { + NFSUNLOCKCLSTATE(); + return (0); } } } @@ -3310,24 +3301,21 @@ nfscl_doclose(vnode_t vp, struct nfsclclient **clpp, NFSPROC_T *p) /* Now process the opens against the server. */ lookformore: - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { - op = LIST_FIRST(&owp->nfsow_open); - while (op != NULL) { - if (op->nfso_fhlen == nfhp->nfh_len && - !NFSBCMP(op->nfso_fh, nfhp->nfh_fh, - nfhp->nfh_len)) { - /* Found an open, close it. */ + LIST_FOREACH(op, NFSCLOPENHASH(clp, nfhp->nfh_fh, nfhp->nfh_len), + nfso_hash) { + if (op->nfso_fhlen == nfhp->nfh_len && + !NFSBCMP(op->nfso_fh, nfhp->nfh_fh, + nfhp->nfh_len)) { + /* Found an open, close it. */ #ifdef DIAGNOSTIC - KASSERT((op->nfso_opencnt == 0), - ("nfscl: bad open cnt on server (%d)", - op->nfso_opencnt)); + KASSERT((op->nfso_opencnt == 0), + ("nfscl: bad open cnt on server (%d)", + op->nfso_opencnt)); #endif - NFSUNLOCKCLSTATE(); - nfsrpc_doclose(VFSTONFS(vp->v_mount), op, p); - NFSLOCKCLSTATE(); - goto lookformore; - } - op = LIST_NEXT(op, nfso_list); + NFSUNLOCKCLSTATE(); + nfsrpc_doclose(VFSTONFS(vp->v_mount), op, p); + NFSLOCKCLSTATE(); + goto lookformore; } } NFSUNLOCKCLSTATE(); @@ -3956,7 +3944,6 @@ nfscl_localconflict(struct nfsclclient *clp, u_int8_t *fhp, int fhlen, struct nfscllock *nlop, u_int8_t *own, struct nfscldeleg *dp, struct nfscllock **lopp) { - struct nfsclowner *owp; struct nfsclopen *op; int ret; @@ -3965,15 +3952,13 @@ nfscl_localconflict(struct nfsclclient *clp, u_int8_t *fhp, int fhlen, if (ret) return (ret); } - 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, fhp, fhlen)) { - ret = nfscl_checkconflict(&op->nfso_lock, nlop, - own, lopp); - if (ret) - return (ret); - } + LIST_FOREACH(op, NFSCLOPENHASH(clp, fhp, fhlen), nfso_hash) { + if (op->nfso_fhlen == fhlen && + !NFSBCMP(op->nfso_fh, fhp, fhlen)) { + ret = nfscl_checkconflict(&op->nfso_lock, nlop, + own, lopp); + if (ret) + return (ret); } } return (0);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106120130.15C1Ugpi025576>