The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=a7de51068502ad1e2851d4a855ed28b27573bb36
commit a7de51068502ad1e2851d4a855ed28b27573bb36 Author: Rick Macklem <[email protected]> AuthorDate: 2024-06-21 22:08:48 +0000 Commit: Rick Macklem <[email protected]> CommitDate: 2024-06-21 22:08:48 +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. MFC after: 1 month --- 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); }
