git: dfaeeacc2cc2 - main - nfsd: Allow a mutex lock for clientID handling
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 22 Jun 2024 22:57:48 UTC
The branch main has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=dfaeeacc2cc29d0497ecd4cd5b7fd0d5ab61fcd5
commit dfaeeacc2cc29d0497ecd4cd5b7fd0d5ab61fcd5
Author: Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-06-22 22:56:40 +0000
Commit: Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2024-06-22 22:56:40 +0000
nfsd: Allow a mutex lock for clientID handling
On Feb. 28, a problem was reported on freebsd-stable@ where a
nfsd thread processing an ExchangeID operation was blocked for
a long time by another nfsd thread performing a copy_file_range.
This occurred because the copy_file_range was taking a long time,
but also because handling a clientID requires that all other nfsd
threads be blocked via an exclusive lock, as required by ExchangeID.
This patch allows clientID handling to be done with only a mutex
held (instead of an exclusive lock that blocks all other nfsd threads)
when vfs.nfsd.enable_locallocks is 0. For the case of
vfs.nfsd.enable_locallocks set to 1, the exclusive lock that
blocks all nfsd threads is still required.
This patch does make changing the value of vfs.nfsd.enable_locallocks
somewhat racy. A future commit will ensure any change is done when
all nfsd threads are blocked to avoid this racyness.
MFC after: 1 month
---
sys/fs/nfsserver/nfs_nfsdstate.c | 245 ++++++++++++++++++++++++++-------------
1 file changed, 162 insertions(+), 83 deletions(-)
diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
index 7a28e51e21fc..6b40e0f64141 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -245,6 +245,45 @@ static void nfsrv_issuedelegation(struct vnode *vp, struct nfsclient *clp,
u_quad_t filerev, uint64_t rdonly, struct nfsstate **new_delegp,
struct nfsstate *new_stp, struct nfslockfile *lfp, uint32_t *rflagsp,
nfsv4stateid_t *delegstateidp);
+static void nfsrv_clientlock(bool mlocked);
+static void nfsrv_clientunlock(bool mlocked);
+
+/*
+ * Lock the client structure, either with the mutex or the exclusive nfsd lock.
+ */
+static void
+nfsrv_clientlock(bool mlocked)
+{
+ int igotlock;
+
+ if (mlocked) {
+ NFSLOCKSTATE();
+ } else {
+ NFSLOCKV4ROOTMUTEX();
+ nfsv4_relref(&nfsv4rootfs_lock);
+ do {
+ igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL,
+ NFSV4ROOTLOCKMUTEXPTR, NULL);
+ } while (!igotlock);
+ NFSUNLOCKV4ROOTMUTEX();
+ }
+}
+
+/*
+ * Unlock the client structure.
+ */
+static void
+nfsrv_clientunlock(bool mlocked)
+{
+
+ if (mlocked) {
+ NFSUNLOCKSTATE();
+ } else {
+ NFSLOCKV4ROOTMUTEX();
+ nfsv4_unlock(&nfsv4rootfs_lock, 1);
+ NFSUNLOCKV4ROOTMUTEX();
+ }
+}
/*
* Scan the client list for a match and either return the current one,
@@ -266,7 +305,10 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
struct sockaddr_in6 *sin6, *rin6;
#endif
struct nfsdsession *sep, *nsep;
- int zapit = 0, gotit, hasstate = 0, igotlock;
+ SVCXPRT *old_xprt;
+ struct nfssessionhead old_sess;
+ int zapit = 0, gotit, hasstate = 0;
+ bool mlocked;
static u_int64_t confirm_index = 0;
/*
@@ -294,14 +336,11 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
*/
new_clp->lc_program = 0;
+ mlocked = true;
+ if (nfsrv_dolocallocks != 0)
+ mlocked = false;
/* Lock out other nfsd threads */
- NFSLOCKV4ROOTMUTEX();
- nfsv4_relref(&nfsv4rootfs_lock);
- do {
- igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL,
- NFSV4ROOTLOCKMUTEXPTR, NULL);
- } while (!igotlock);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientlock(mlocked);
/*
* Search for a match in the client list.
@@ -318,6 +357,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
if (gotit == 0)
i++;
}
+ old_xprt = NULL;
if (!gotit ||
(clp->lc_flags & (LCL_NEEDSCONFIRM | LCL_ADMINREVOKED))) {
if ((nd->nd_flag & ND_NFSV41) != 0 && confirmp->lval[1] != 0) {
@@ -325,9 +365,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
* For NFSv4.1, if confirmp->lval[1] is non-zero, the
* client is trying to update a confirmed clientid.
*/
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
confirmp->lval[1] = 0;
error = NFSERR_NOENT;
goto out;
@@ -337,7 +375,10 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
*/
if (i != nfsrv_clienthashsize) {
LIST_REMOVE(clp, lc_hash);
- nfsrv_cleanclient(clp, p, false, NULL);
+ if (mlocked)
+ nfsrv_cleanclient(clp, p, true, &old_xprt);
+ else
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
zapit = 1;
@@ -372,11 +413,12 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
NFSD_VNET(nfsstatsv1_p)->srvclients++;
nfsrv_openpluslock++;
nfsrv_clients++;
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
- if (zapit)
+ nfsrv_clientunlock(mlocked);
+ if (zapit != 0) {
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
nfsrv_zapclient(clp, p);
+ }
*new_clpp = NULL;
goto out;
}
@@ -390,7 +432,10 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
*/
if (clp->lc_expiry < NFSD_MONOSEC &&
(!LIST_EMPTY(&clp->lc_open) || !LIST_EMPTY(&clp->lc_deleg))) {
- nfsrv_cleanclient(clp, p, false, NULL);
+ if (mlocked)
+ nfsrv_cleanclient(clp, p, true, &old_xprt);
+ else
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
}
@@ -435,9 +480,9 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
break;
#endif
}
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
error = NFSERR_CLIDINUSE;
goto out;
}
@@ -452,13 +497,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
*/
LIST_REMOVE(clp, lc_hash);
- /* Get rid of all sessions on this clientid. */
- LIST_FOREACH_SAFE(sep, &clp->lc_session, sess_list, nsep) {
- ret = nfsrv_freesession(NULL, sep, NULL, false, NULL);
- if (ret != 0)
- printf("nfsrv_setclient: verifier changed free"
- " session failed=%d\n", ret);
- }
+ LIST_NEWHEAD(&old_sess, &clp->lc_session, sess_list);
new_clp->lc_flags |= LCL_NEEDSCONFIRM;
if ((nd->nd_flag & ND_NFSV41) != 0) {
@@ -502,21 +541,31 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
NFSD_VNET(nfsstatsv1_p)->srvclients++;
nfsrv_openpluslock++;
nfsrv_clients++;
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ if (!mlocked) {
+ nfsrv_clientunlock(mlocked);
+ NFSLOCKSTATE();
+ }
/*
* Must wait until any outstanding callback on the old clp
* completes.
*/
- NFSLOCKSTATE();
while (clp->lc_cbref) {
clp->lc_flags |= LCL_WAKEUPWANTED;
(void)mtx_sleep(clp, NFSSTATEMUTEXPTR, PZERO - 1,
"nfsd clp", 10 * hz);
}
NFSUNLOCKSTATE();
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
+ /* Get rid of all sessions on this clientid. */
+ LIST_FOREACH_SAFE(sep, &old_sess, sess_list, nsep) {
+ ret = nfsrv_freesession(NULL, sep, NULL, false, NULL);
+ if (ret != 0)
+ printf("nfsrv_setclient: verifier changed free"
+ " session failed=%d\n", ret);
+ }
+
nfsrv_zapclient(clp, p);
*new_clpp = NULL;
goto out;
@@ -568,24 +617,31 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
nfsrv_openpluslock++;
nfsrv_clients++;
}
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ if (!mlocked)
+ nfsrv_clientunlock(mlocked);
if ((nd->nd_flag & ND_NFSV41) == 0) {
/*
* Must wait until any outstanding callback on the old clp
* completes.
*/
- NFSLOCKSTATE();
+ if (!mlocked)
+ NFSLOCKSTATE();
while (clp->lc_cbref) {
clp->lc_flags |= LCL_WAKEUPWANTED;
(void)mtx_sleep(clp, NFSSTATEMUTEXPTR, PZERO - 1,
"nfsdclp", 10 * hz);
}
NFSUNLOCKSTATE();
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
nfsrv_zapclient(clp, p);
*new_clpp = NULL;
+ } else {
+ if (mlocked)
+ NFSUNLOCKSTATE();
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
}
out:
@@ -605,11 +661,13 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
struct nfsstate *stp;
int i;
struct nfsclienthashhead *hp;
- int error = 0, igotlock, doneok;
+ int error = 0, doneok, igotlock;
struct nfssessionhash *shp;
struct nfsdsession *sep;
uint64_t sessid[2];
- bool sess_replay;
+ CLIENT *client;
+ SVCXPRT *old_xprt;
+ bool mlocked, sess_replay;
static uint64_t next_sess = 0;
if (clpp)
@@ -626,13 +684,27 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
* already held. Otherwise, we need to get either that or,
* for the case of Confirm, lock out the nfsd threads.
*/
+ client = NULL;
+ old_xprt = NULL;
+ mlocked = true;
+ if (nfsrv_dolocallocks != 0)
+ mlocked = false;
if (opflags & CLOPS_CONFIRM) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_relref(&nfsv4rootfs_lock);
- do {
- igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL,
- NFSV4ROOTLOCKMUTEXPTR, NULL);
- } while (!igotlock);
+ if (nsep != NULL &&
+ (nsep->sess_crflags & NFSV4CRSESS_CONNBACKCHAN) != 0)
+ client = (struct __rpc_client *)
+ clnt_bck_create(nd->nd_xprt->xp_socket,
+ cbprogram, NFSV4_CBVERS);
+ if (mlocked) {
+ nfsrv_clientlock(mlocked);
+ } else {
+ NFSLOCKV4ROOTMUTEX();
+ nfsv4_relref(&nfsv4rootfs_lock);
+ do {
+ igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1,
+ NULL, NFSV4ROOTLOCKMUTEXPTR, NULL);
+ } while (!igotlock);
+ }
/*
* Create a new sessionid here, since we need to do it where
* there is a mutex held to serialize update of next_sess.
@@ -641,7 +713,8 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
sessid[0] = ++next_sess;
sessid[1] = clientid.qval;
}
- NFSUNLOCKV4ROOTMUTEX();
+ if (!mlocked)
+ NFSUNLOCKV4ROOTMUTEX();
} else if (opflags != CLOPS_RENEW) {
NFSLOCKSTATE();
}
@@ -678,9 +751,9 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
}
if (error) {
if (opflags & CLOPS_CONFIRM) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
+ if (client != NULL)
+ CLNT_RELEASE(client);
} else if (opflags != CLOPS_RENEW) {
NFSUNLOCKSTATE();
}
@@ -725,7 +798,10 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
* for an Open with CLAIM_DELEGATE_PREV unless in
* grace, but get rid of the rest of the state.
*/
- nfsrv_cleanclient(clp, p, false, NULL);
+ if (mlocked)
+ nfsrv_cleanclient(clp, p, true, &old_xprt);
+ else
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_olddeleg);
if (nfsrv_checkgrace(nd, clp, 0)) {
/* In grace, so just delete delegations */
@@ -749,10 +825,10 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
/* Hold a reference on the xprt for a backchannel. */
if ((nsep->sess_crflags & NFSV4CRSESS_CONNBACKCHAN)
!= 0 && !sess_replay) {
- if (clp->lc_req.nr_client == NULL)
- clp->lc_req.nr_client = (struct __rpc_client *)
- clnt_bck_create(nd->nd_xprt->xp_socket,
- cbprogram, NFSV4_CBVERS);
+ if (clp->lc_req.nr_client == NULL) {
+ clp->lc_req.nr_client = client;
+ client = NULL;
+ }
if (clp->lc_req.nr_client != NULL) {
SVC_ACQUIRE(nd->nd_xprt);
CLNT_ACQUIRE(clp->lc_req.nr_client);
@@ -769,13 +845,15 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
NFSX_V4SESSIONID);
if (!sess_replay) {
shp = NFSSESSIONHASH(nsep->sess_sessionid);
- NFSLOCKSTATE();
+ if (!mlocked)
+ NFSLOCKSTATE();
NFSLOCKSESSION(shp);
LIST_INSERT_HEAD(&shp->list, nsep, sess_hash);
LIST_INSERT_HEAD(&clp->lc_session, nsep, sess_list);
nsep->sess_clp = clp;
NFSUNLOCKSESSION(shp);
- NFSUNLOCKSTATE();
+ if (!mlocked)
+ NFSUNLOCKSTATE();
}
}
}
@@ -809,9 +887,11 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
clp->lc_expiry = nfsrv_leaseexpiry();
}
if (opflags & CLOPS_CONFIRM) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
+ if (client != NULL)
+ CLNT_RELEASE(client);
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
} else if (opflags != CLOPS_RENEW) {
NFSUNLOCKSTATE();
}
@@ -831,21 +911,20 @@ nfsrv_destroyclient(struct nfsrv_descript *nd, nfsquad_t clientid, NFSPROC_T *p)
{
struct nfsclient *clp;
struct nfsclienthashhead *hp;
- int error = 0, i, igotlock;
+ SVCXPRT *old_xprt;
+ int error = 0, i;
+ bool mlocked;
if (NFSD_VNET(nfsrvboottime) != clientid.lval[0]) {
error = NFSERR_STALECLIENTID;
goto out;
}
+ mlocked = true;
+ if (nfsrv_dolocallocks != 0)
+ mlocked = false;
/* Lock out other nfsd threads */
- NFSLOCKV4ROOTMUTEX();
- nfsv4_relref(&nfsv4rootfs_lock);
- do {
- igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL,
- NFSV4ROOTLOCKMUTEXPTR, NULL);
- } while (igotlock == 0);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientlock(mlocked);
hp = NFSCLIENTHASH(clientid);
LIST_FOREACH(clp, hp, lc_hash) {
@@ -853,9 +932,7 @@ nfsrv_destroyclient(struct nfsrv_descript *nd, nfsquad_t clientid, NFSPROC_T *p)
break;
}
if (clp == NULL) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
/* Just return ok, since it is gone. */
goto out;
}
@@ -863,9 +940,7 @@ nfsrv_destroyclient(struct nfsrv_descript *nd, nfsquad_t clientid, NFSPROC_T *p)
/* Check for the SP4_MACH_CRED case. */
error = nfsrv_checkmachcred(NFSV4OP_DESTROYCLIENTID, nd, clp);
if (error != 0) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
goto out;
}
@@ -878,28 +953,28 @@ nfsrv_destroyclient(struct nfsrv_descript *nd, nfsquad_t clientid, NFSPROC_T *p)
/* Scan for state on the clientid. */
for (i = 0; i < nfsrv_statehashsize; i++)
if (!LIST_EMPTY(&clp->lc_stateid[i])) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
error = NFSERR_CLIENTIDBUSY;
goto out;
}
if (!LIST_EMPTY(&clp->lc_session) || !LIST_EMPTY(&clp->lc_deleg)) {
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
error = NFSERR_CLIENTIDBUSY;
goto out;
}
/* Destroy the clientid and return ok. */
- nfsrv_cleanclient(clp, p, false, NULL);
+ old_xprt = NULL;
+ if (mlocked)
+ nfsrv_cleanclient(clp, p, true, &old_xprt);
+ else
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
LIST_REMOVE(clp, lc_hash);
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
+ nfsrv_clientunlock(mlocked);
+ if (old_xprt != NULL)
+ SVC_RELEASE(old_xprt);
nfsrv_zapclient(clp, p);
out:
NFSEXITCODE2(error, nd);
@@ -1388,8 +1463,12 @@ nfsrv_cleanclient(struct nfsclient *clp, NFSPROC_T *p, bool locked,
struct nfsstate *stp, *nstp;
struct nfsdsession *sep, *nsep;
- LIST_FOREACH_SAFE(stp, &clp->lc_open, ls_list, nstp)
- nfsrv_freeopenowner(stp, 1, p);
+ LIST_FOREACH_SAFE(stp, &clp->lc_open, ls_list, nstp) {
+ if (locked)
+ nfsrv_freeopenowner(stp, 0, p);
+ else
+ nfsrv_freeopenowner(stp, 1, p);
+ }
if ((clp->lc_flags & LCL_ADMINREVOKED) == 0)
LIST_FOREACH_SAFE(sep, &clp->lc_session, sess_list, nsep)
(void)nfsrv_freesession(NULL, sep, NULL, locked,