[Bug 299225] nfscl: one NFSv4.1 session (struct nfsclds) leaked per recovery until unmount

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=299225

            Bug ID: 299225
           Summary: nfscl: one NFSv4.1 session (struct nfsclds) leaked per
                    recovery until unmount
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: olivier@freebsd.org

Created attachment 275504
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=275504&action=edit
patch

On a successful recovery, nfsrpc_setclient() inserts the new session at the
head of nm_sess and only marks the old one defunct ("The old sessions cannot
be safely free'd here, since they may still be used by in-progress RPCs").
Nothing frees defunct sessions later: nfscl_freenfsclds() is only called for
them at unmount (nfs_clvfsops.c, nfs_unmount paths). The renew thread skips
defunct entries but keeps walking them.

So a long-lived mount whose server forces frequent recoveries grows nm_sess
and M_NFSCLDS without bound.

Observed (3 mounts on Amazon EFS, which drops sessions on every connection
reset; 2.5 days uptime):

  $ vmstat -m | grep NFSCL
     NFSCL client    3 24576 13960 8192
    NFSCL session  256 262144  834 1024
  $ nfsstat -c -E
     ExchangeId   CreateSess  DestroySess  DestroyClId
            374          834          114          114

  NFSCL session in use: 37 -> 64 (8h) -> 256 (+19h), still 3 mounts.
  1 KB per recovery here; the list walks grow with it.

Fix (attached, session-leak.patch, against main a2fbd988638e):
- nfscl_reqstart() takes a reference (nfsess_refcnt, NFSLOCKMNT) on the MDS
  session it hands to nfsv4_setsequence() and sets ND_HOLDSESS; slot waiters
  therefore hold it. A flag, not nd_sep alone, because nfsrv_getuser()
  builds its nd on the stack without nfscl_reqstart().
- newnfs_request() becomes a wrapper around the old body (now
  newnfs_dorequest()) that drops the reference on every return path; the
  "Badsession looping" retry moves it to the new session.
- nfsrpc_setclient() stamps nfsess_retired when a recovery replaces the
  session. nfscl_freeretiredsess(), called from nfscl_renewthread(), frees
  retired sessions with no reference after 2 * nfsc_renew. The live MDS
  session and DS sessions are never freed there.

-- 
You are receiving this mail because:
You are the assignee for the bug.