Skip to content

Commit

Permalink
nfsd: Fix nfsrv_cleanclient so that it can be called with a mutex
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bsdjhb
Rick Macklem authored and bsdjhb committed Oct 24, 2024
2 parents b947365 + a7de510 commit 7de5544
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 24 deletions.
2 changes: 1 addition & 1 deletion sys/fs/nfs/nfsrvstate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);

/*
Expand Down
2 changes: 1 addition & 1 deletion sys/fs/nfsserver/nfs_nfsdsocket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
55 changes: 33 additions & 22 deletions sys/fs/nfsserver/nfs_nfsdstate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

/*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -6469,12 +6471,13 @@ nfsrv_bindconnsess(struct nfsrv_descript *nd, uint8_t *sessionid, int *foreaftp)
*/
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);
Expand All @@ -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);
}
Expand Down

0 comments on commit 7de5544

Please sign in to comment.