git: c664b786ccd1 - stable/13 - nfsd: Fix nfsrv_cleanclient so that it can be called with a mutex
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 22 Jul 2024 01:39:23 UTC
The branch stable/13 has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=c664b786ccd18bd186c59279e26fd19c6a212be4
commit c664b786ccd18bd186c59279e26fd19c6a212be4
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:30:11 +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 | 53 +++++++++++++++++++++++----------------
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/sys/fs/nfs/nfsrvstate.h b/sys/fs/nfs/nfsrvstate.h
index 3652e2c2db2f..c196f43aac04 100644
--- a/sys/fs/nfs/nfsrvstate.h
+++ b/sys/fs/nfs/nfsrvstate.h
@@ -331,7 +331,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 da92b7b0047c..94369c575e52 100644
--- a/sys/fs/nfsserver/nfs_nfsdsocket.c
+++ b/sys/fs/nfsserver/nfs_nfsdsocket.c
@@ -792,7 +792,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 06d0b79f10d5..911f5053bd3c 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -203,7 +203,8 @@ static void nfsrv_locallock_commit(struct nfslockfile *lfp, int flags,
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 nfsdsession *sep, uint8_t *sessionid);
+static int nfsrv_freesession(struct nfsdsession *sep, 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);
@@ -329,7 +330,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;
@@ -382,7 +383,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);
}
@@ -445,7 +446,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(sep, NULL);
+ ret = nfsrv_freesession(sep, NULL, false, NULL);
if (ret != 0)
printf("nfsrv_setclient: verifier changed free"
" session failed=%d\n", ret);
@@ -715,7 +716,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 */
@@ -874,7 +875,7 @@ nfsrv_destroyclient(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);
@@ -943,7 +944,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();
@@ -1363,7 +1364,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;
@@ -1372,7 +1374,7 @@ 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(sep, NULL);
+ (void)nfsrv_freesession(sep, NULL, locked, old_xprtp);
}
/*
@@ -4603,7 +4605,7 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
if (procnum != NFSV4PROC_CBNULL)
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
- nfsrv_freesession(sep, NULL);
+ nfsrv_freesession(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);
@@ -4652,7 +4654,7 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
}
- nfsrv_freesession(sep, NULL);
+ nfsrv_freesession(sep, NULL, false, NULL);
} else
error = newnfs_request(nd, NULL, clp, &clp->lc_req,
NULL, NULL, cred, clp->lc_program,
@@ -5250,7 +5252,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);
@@ -5442,7 +5444,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);
@@ -6243,7 +6245,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);
@@ -6456,7 +6458,7 @@ nfsrv_destroysession(struct nfsrv_descript *nd, uint8_t *sessionid)
} while (igotlock == 0);
NFSUNLOCKV4ROOTMUTEX();
- error = nfsrv_freesession(NULL, sessionid);
+ error = nfsrv_freesession(NULL, sessionid, false, NULL);
if (error == 0 && samesess != 0)
nd->nd_flag &= ~ND_HASSEQUENCE;
@@ -6547,12 +6549,14 @@ nfsrv_bindconnsess(struct nfsrv_descript *nd, uint8_t *sessionid, int *foreaftp)
* Free up a session structure.
*/
static int
-nfsrv_freesession(struct nfsdsession *sep, uint8_t *sessionid)
+nfsrv_freesession(struct nfsdsession *sep, 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);
@@ -6565,21 +6569,28 @@ nfsrv_freesession(struct nfsdsession *sep, uint8_t *sessionid)
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);
}