git: 7ac2b39c59bd - stable/14 - nfsd: Fix nfsrv_cleanclient so that it can be called with a mutex
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 21 Jul 2024 23:10:23 UTC
The branch stable/14 has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=7ac2b39c59bda30d42478ed6ca09baf588bff773
commit 7ac2b39c59bda30d42478ed6ca09baf588bff773
Author: Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-06-21 22:08:48 +0000
Commit: Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2024-07-21 23:06:02 +0000
nfsd: Fix nfsrv_cleanclient so that it can be called with a mutex
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 adds two arguments to nfsv4_cleanclient() so that it
can optionally be called with a mutex held. For this patch, the
first of these arguments is "false" and, as such, there is no
change in semantics. However, this change will allow a future
commit to modify handling of the clientID so that it can be done
with a mutex held while other nfsd threads continue to process
NFS RPCs.
(cherry picked from commit a7de51068502ad1e2851d4a855ed28b27573bb36)
---
sys/fs/nfs/nfsrvstate.h | 2 +-
sys/fs/nfsserver/nfs_nfsdsocket.c | 2 +-
sys/fs/nfsserver/nfs_nfsdstate.c | 55 +++++++++++++++++++++++----------------
3 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/sys/fs/nfs/nfsrvstate.h b/sys/fs/nfs/nfsrvstate.h
index da214ae9d4e9..cc19ed6fa1d2 100644
--- a/sys/fs/nfs/nfsrvstate.h
+++ b/sys/fs/nfs/nfsrvstate.h
@@ -333,7 +333,7 @@ struct nfsf_rec {
u_int32_t numboots; /* Number of boottimes */
};
-void nfsrv_cleanclient(struct nfsclient *, NFSPROC_T *);
+void nfsrv_cleanclient(struct nfsclient *, NFSPROC_T *, bool, SVCXPRT **);
void nfsrv_freedeleglist(struct nfsstatehead *);
/*
diff --git a/sys/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsdsocket.c
index 1f50634405d0..df0c0edd1b59 100644
--- a/sys/fs/nfsserver/nfs_nfsdsocket.c
+++ b/sys/fs/nfsserver/nfs_nfsdsocket.c
@@ -797,7 +797,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
!LIST_EMPTY(&clp->lc_deleg))
nfsrv_writestable(clp->lc_id,
clp->lc_idlen, NFSNST_REVOKE, p);
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
LIST_REMOVE(clp, lc_hash);
diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
index ce3f3481f04a..7a28e51e21fc 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -204,7 +204,7 @@ static void nfsrv_locklf(struct nfslockfile *lfp);
static void nfsrv_unlocklf(struct nfslockfile *lfp);
static struct nfsdsession *nfsrv_findsession(uint8_t *sessionid);
static int nfsrv_freesession(struct nfsrv_descript *nd, struct nfsdsession *sep,
- uint8_t *sessionid);
+ uint8_t *sessionid, bool locked, SVCXPRT **old_xprtp);
static int nfsv4_setcbsequence(struct nfsrv_descript *nd, struct nfsclient *clp,
int dont_replycache, struct nfsdsession **sepp, int *slotposp);
static int nfsv4_getcbsession(struct nfsclient *clp, struct nfsdsession **sepp);
@@ -337,7 +337,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
*/
if (i != nfsrv_clienthashsize) {
LIST_REMOVE(clp, lc_hash);
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
zapit = 1;
@@ -390,7 +390,7 @@ 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);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
}
@@ -454,7 +454,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
/* Get rid of all sessions on this clientid. */
LIST_FOREACH_SAFE(sep, &clp->lc_session, sess_list, nsep) {
- ret = nfsrv_freesession(NULL, sep, NULL);
+ ret = nfsrv_freesession(NULL, sep, NULL, false, NULL);
if (ret != 0)
printf("nfsrv_setclient: verifier changed free"
" session failed=%d\n", ret);
@@ -725,7 +725,7 @@ 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);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_olddeleg);
if (nfsrv_checkgrace(nd, clp, 0)) {
/* In grace, so just delete delegations */
@@ -893,7 +893,7 @@ nfsrv_destroyclient(struct nfsrv_descript *nd, nfsquad_t clientid, NFSPROC_T *p)
}
/* Destroy the clientid and return ok. */
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
LIST_REMOVE(clp, lc_hash);
@@ -962,7 +962,7 @@ nfsrv_adminrevoke(struct nfsd_clid *revokep, NFSPROC_T *p)
*/
clp->lc_flags &= ~LCL_CALLBACKSON;
clp->lc_flags |= LCL_ADMINREVOKED;
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
NFSLOCKV4ROOTMUTEX();
@@ -1382,7 +1382,8 @@ nfsrv_servertimer(void *arg __unused)
* there are no other active nfsd threads.
*/
void
-nfsrv_cleanclient(struct nfsclient *clp, NFSPROC_T *p)
+nfsrv_cleanclient(struct nfsclient *clp, NFSPROC_T *p, bool locked,
+ SVCXPRT **old_xprtp)
{
struct nfsstate *stp, *nstp;
struct nfsdsession *sep, *nsep;
@@ -1391,7 +1392,8 @@ nfsrv_cleanclient(struct nfsclient *clp, NFSPROC_T *p)
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);
+ (void)nfsrv_freesession(NULL, sep, NULL, locked,
+ old_xprtp);
}
/*
@@ -4499,7 +4501,7 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
if (procnum != NFSV4PROC_CBNULL)
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
- nfsrv_freesession(NULL, sep, NULL);
+ nfsrv_freesession(NULL, sep, NULL, false, NULL);
} else if (nd->nd_procnum == NFSV4PROC_CBNULL)
error = newnfs_connect(NULL, &clp->lc_req, cred,
NULL, 1, dotls, &clp->lc_req.nr_client);
@@ -4548,7 +4550,7 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
}
- nfsrv_freesession(NULL, sep, NULL);
+ nfsrv_freesession(NULL, sep, NULL, false, NULL);
} else
error = newnfs_request(nd, NULL, clp, &clp->lc_req,
NULL, NULL, cred, clp->lc_program,
@@ -5151,7 +5153,7 @@ nfsrv_clientconflict(struct nfsclient *clp, int *haslockp, vnode_t vp,
*/
nfsrv_writestable(clp->lc_id, clp->lc_idlen, NFSNST_REVOKE, p);
nfsrv_backupstable();
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
LIST_REMOVE(clp, lc_hash);
@@ -5343,7 +5345,7 @@ nfsrv_delegconflict(struct nfsstate *stp, int *haslockp, NFSPROC_T *p,
nfsrv_writestable(clp->lc_id, clp->lc_idlen, NFSNST_REVOKE, p);
nfsrv_backupstable();
if (clp->lc_expiry < NFSD_MONOSEC) {
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
LIST_REMOVE(clp, lc_hash);
@@ -6150,7 +6152,7 @@ nfsrv_throwawayallstate(NFSPROC_T *p)
for (i = 0; i < nfsrv_clienthashsize; i++) {
LIST_FOREACH_SAFE(clp, &NFSD_VNET(nfsclienthash)[i], lc_hash,
nclp) {
- nfsrv_cleanclient(clp, p);
+ nfsrv_cleanclient(clp, p, false, NULL);
nfsrv_freedeleglist(&clp->lc_deleg);
nfsrv_freedeleglist(&clp->lc_olddeleg);
free(clp->lc_stateid, M_NFSDCLIENT);
@@ -6373,7 +6375,7 @@ nfsrv_destroysession(struct nfsrv_descript *nd, uint8_t *sessionid)
} while (igotlock == 0);
NFSUNLOCKV4ROOTMUTEX();
- error = nfsrv_freesession(nd, NULL, sessionid);
+ error = nfsrv_freesession(nd, NULL, sessionid, false, NULL);
if (error == 0 && samesess != 0)
nd->nd_flag &= ~ND_HASSEQUENCE;
@@ -6469,12 +6471,13 @@ out:
*/
static int
nfsrv_freesession(struct nfsrv_descript *nd, struct nfsdsession *sep,
- uint8_t *sessionid)
+ uint8_t *sessionid, bool locked, SVCXPRT **old_xprtp)
{
struct nfssessionhash *shp;
int i;
- NFSLOCKSTATE();
+ if (!locked)
+ NFSLOCKSTATE();
if (sep == NULL) {
shp = NFSSESSIONHASH(sessionid);
NFSLOCKSESSION(shp);
@@ -6488,28 +6491,36 @@ nfsrv_freesession(struct nfsrv_descript *nd, struct nfsdsession *sep,
if (nd != NULL && nfsrv_checkmachcred(NFSV4OP_DESTROYSESSION,
nd, sep->sess_clp) != 0) {
NFSUNLOCKSESSION(shp);
- NFSUNLOCKSTATE();
+ if (!locked)
+ NFSUNLOCKSTATE();
return (NFSERR_AUTHERR | AUTH_TOOWEAK);
}
sep->sess_refcnt--;
if (sep->sess_refcnt > 0) {
NFSUNLOCKSESSION(shp);
- NFSUNLOCKSTATE();
+ if (!locked)
+ NFSUNLOCKSTATE();
return (NFSERR_BACKCHANBUSY);
}
LIST_REMOVE(sep, sess_hash);
LIST_REMOVE(sep, sess_list);
}
NFSUNLOCKSESSION(shp);
- NFSUNLOCKSTATE();
+ if (!locked)
+ NFSUNLOCKSTATE();
if (sep == NULL)
return (NFSERR_BADSESSION);
for (i = 0; i < NFSV4_SLOTS; i++)
if (sep->sess_slots[i].nfssl_reply != NULL)
m_freem(sep->sess_slots[i].nfssl_reply);
- if (sep->sess_cbsess.nfsess_xprt != NULL)
- SVC_RELEASE(sep->sess_cbsess.nfsess_xprt);
+ if (!locked) {
+ if (sep->sess_cbsess.nfsess_xprt != NULL)
+ SVC_RELEASE(sep->sess_cbsess.nfsess_xprt);
+ if (old_xprtp != NULL)
+ *old_xprtp = NULL;
+ } else if (old_xprtp != NULL)
+ *old_xprtp = sep->sess_cbsess.nfsess_xprt;
free(sep, M_NFSDSESSION);
return (0);
}