d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nfscl: Use hash lists to improve expected search performance for opens

Browse Source 
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 3f7e14ad93 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
This commit is contained in:
Rick Macklem 2021-05-25 14:19:29 -07:00
parent 323a4e2c4e
commit 724072ab1d
1 changed files with 58 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
sys/fs/nfsclient/nfs_clstate.c
View File

@ -100,8 +100,9 @@ int nfscl_layouthighwater = NFSCLLAYOUTHIGHWATER;
static int nfscl_delegcnt = 0; static int nfscl_delegcnt = 0;
static int nfscl_layoutcnt = 0; static int nfscl_layoutcnt = 0;
static int nfscl_getopen(struct nfsclownerhead *, u_int8_t *, int, u_int8_t *, static int nfscl_getopen(struct nfsclownerhead *, struct nfsclopenhash *,
u_int8_t *, u_int32_t, struct nfscllockowner **, struct nfsclopen **); 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 *, static bool nfscl_checkown(struct nfsclowner *, struct nfsclopen *, uint8_t *,
uint8_t *, struct nfscllockowner **, struct nfsclopen **, uint8_t *, struct nfscllockowner **, struct nfsclopen **,
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) void **lckpp)
{ {
struct nfsclclient *clp; struct nfsclclient *clp;
struct nfsclowner *owp;
struct nfsclopen *op = NULL, *top; struct nfsclopen *op = NULL, *top;
struct nfsclopenhash *oph;
struct nfscllockowner *lp; struct nfscllockowner *lp;
struct nfscldeleg *dp; struct nfscldeleg *dp;
struct nfsnode *np; struct nfsnode *np;
@ -587,8 +588,8 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
else else
nfscl_filllockowner(p->td_proc, own, F_POSIX); nfscl_filllockowner(p->td_proc, own, F_POSIX);
lp = NULL; lp = NULL;
error = nfscl_getopen(&clp->nfsc_owner, nfhp, fhlen, own, own, error = nfscl_getopen(NULL, clp->nfsc_openhash, nfhp, fhlen,
mode, &lp, &op); own, own, mode, &lp, &op);
if (error == 0 && lp != NULL && fords == 0) { if (error == 0 && lp != NULL && fords == 0) {
/* Don't return a lock stateid for a DS. */ /* Don't return a lock stateid for a DS. */
stateidp->seqid = 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. */ /* If not found, just look for any OpenOwner that will work. */
top = NULL; top = NULL;
done = false; done = false;
LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { oph = NFSCLOPENHASH(clp, nfhp, fhlen);
LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { LIST_FOREACH(op, oph, nfso_hash) {
if (op->nfso_fhlen == fhlen && if (op->nfso_fhlen == fhlen &&
!NFSBCMP(op->nfso_fh, nfhp, fhlen)) { !NFSBCMP(op->nfso_fh, nfhp, fhlen)) {
if (top == NULL && (op->nfso_mode & if (top == NULL && (op->nfso_mode &
NFSV4OPEN_ACCESSWRITE) != 0 && NFSV4OPEN_ACCESSWRITE) != 0 &&
(mode & NFSV4OPEN_ACCESSREAD) != 0) (mode & NFSV4OPEN_ACCESSREAD) != 0)
top = op; top = op;
if ((mode & op->nfso_mode) == mode) { if ((mode & op->nfso_mode) == mode) {
done = true; /* LRU order the hash list. */
break; LIST_REMOVE(op, nfso_hash);
} LIST_INSERT_HEAD(oph, op, nfso_hash);
done = true;
break;
} }
} }
if (done)
break;
} }
if (!done) { if (!done) {
NFSCL_DEBUG(2, "openmode top=%p\n", top); 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. * Search for a matching file, mode and, optionally, lockowner.
*/ */
static int static int
nfscl_getopen(struct nfsclownerhead *ohp, u_int8_t *nfhp, int fhlen, nfscl_getopen(struct nfsclownerhead *ohp, struct nfsclopenhash *ohashp,
u_int8_t *openown, u_int8_t *lockown, u_int32_t mode, u_int8_t *nfhp, int fhlen, u_int8_t *openown, u_int8_t *lockown,
struct nfscllockowner **lpp, struct nfsclopen **opp) u_int32_t mode, struct nfscllockowner **lpp, struct nfsclopen **opp)
{ {
struct nfsclowner *owp; struct nfsclowner *owp;
struct nfsclopen *op, *rop, *rop2; struct nfsclopen *op, *rop, *rop2;
struct nfsclopenhash *oph;
bool keep_looping; bool keep_looping;
KASSERT(ohp == NULL || ohashp == NULL, ("nfscl_getopen: "
"only one of ohp and ohashp can be set"));
if (lpp != NULL) if (lpp != NULL)
*lpp = NULL; *lpp = NULL;
/* /*
@ -679,19 +683,38 @@ nfscl_getopen(struct nfsclownerhead *ohp, u_int8_t *nfhp, int fhlen,
rop2 = NULL; rop2 = NULL;
keep_looping = true; keep_looping = true;
/* Search the client list */ /* Search the client list */
LIST_FOREACH(owp, ohp, nfsow_list) { if (ohashp == NULL) {
/* and look for the correct open */ /* Search the local opens on the delegation. */
LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { LIST_FOREACH(owp, ohp, nfsow_list) {
if (op->nfso_fhlen == fhlen && /* and look for the correct open */
!NFSBCMP(op->nfso_fh, nfhp, fhlen) LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
&& (op->nfso_mode & mode) == mode) if (op->nfso_fhlen == fhlen &&
keep_looping = nfscl_checkown(owp, op, openown, !NFSBCMP(op->nfso_fh, nfhp, fhlen)
lockown, lpp, &rop, &rop2); && (op->nfso_mode & mode) == mode)
keep_looping = nfscl_checkown(owp, op, openown,
lockown, lpp, &rop, &rop2);
if (!keep_looping)
break;
}
if (!keep_looping) if (!keep_looping)
break; break;
} }
if (!keep_looping) } else {
break; /* 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(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 (rop == NULL) if (rop == NULL)
rop = rop2; rop = rop2;
@ -1090,10 +1113,10 @@ nfscl_getbytelock(vnode_t vp, u_int64_t off, u_int64_t len,
} }
if (dp != NULL) { if (dp != NULL) {
/* Now, find an open and maybe a lockowner. */ /* 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); np->n_fhp->nfh_len, openownp, ownp, mode, NULL, &op);
if (ret) 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, np->n_fhp->nfh_fh, np->n_fhp->nfh_len, openownp,
ownp, mode, NULL, &op); ownp, mode, NULL, &op);
if (!ret) { 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. * 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, np->n_fhp->nfh_fh, np->n_fhp->nfh_len, openownp,
ownp, mode, &lp, &op); ownp, mode, &lp, &op);
if (!error) if (!error)