From owner-dev-commits-src-main@freebsd.org Tue May 25 21:25:40 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id CBF53654E9D; Tue, 25 May 2021 21:25:40 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FqRvS5RLwz4cCM; Tue, 25 May 2021 21:25:40 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 9EBBB24B35; Tue, 25 May 2021 21:25:40 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 14PLPexY095472; Tue, 25 May 2021 21:25:40 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14PLPemW095471; Tue, 25 May 2021 21:25:40 GMT (envelope-from git) Date: Tue, 25 May 2021 21:25:40 GMT Message-Id: <202105252125.14PLPemW095471@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: 724072ab1d58 - main - nfscl: Use hash lists to improve expected search performance for opens MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 724072ab1d588677a83a5a5011b5ad9ff5d56538 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 May 2021 21:25:40 -0000 The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=724072ab1d588677a83a5a5011b5ad9ff5d56538 commit 724072ab1d588677a83a5a5011b5ad9ff5d56538 Author: Rick Macklem AuthorDate: 2021-05-25 21:19:29 +0000 Commit: Rick Macklem 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)