FBSD: Give 0 'rootrefs' to vflush on unmount 09/13709/4
authorAndrew Deason <adeason@dson.org>
Sun, 21 Jul 2019 04:09:27 +0000 (23:09 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 23 Aug 2019 03:12:12 +0000 (23:12 -0400)
commitd13b647aa392e1d802be1023930a8e1a07fb11ab
tree7bdc773a9f88cc433189c79f1f4c894c1a9eb8c6
parentf5acf1b1bfe940faf0a6f4bd11c55d6c90f60242
FBSD: Give 0 'rootrefs' to vflush on unmount

Currently, in afs_unmount, we give vflush a 'rootrefs' arg of 1,
indicating that we hold 1 reference on the root vnode. But ever since
commit 6eb1088a (freebsd: properly track vcache references), we drop
the ref for the root vnode at the beginning of this function.

What happens currently in afs_unmount for a normal successful umount
is something like this (at least, on FreeBSD 11.2-RELEASE):

- We afs_PutVCache the afs_globalVp vcache, reducing its v_usecount
  and v_holdcnt to 0, and afs_globalVp is set to NULL.

- vflush calls afs_root() to get the root vnode, which sees that
  afs_globalVp is NULL, and so calls afs_GetVCache for the root fid
  and returns it (and sets afs_globalVp to that vcache), with a
  v_usecount of 1.

- vflush tries to vgonel() all of our vnodes, which calls our
  afs_vop_reclaim, which calls afs_FlushVCache(). For the root vnode
  specifically, vflush() sees that v_usecount is nonzero, and so skips
  calling vgonel() at first, but later calls vgone() on it
  specifically because we gave a nonzero 'rootrefs'. The resulting
  afs_FlushVCache() for the root vnode fails, because the root vnode's
  v_usecount is still 1. Since a failure from afs_vop_reclaim would
  cause a panic, we just log a warning and try to continue on anyway.

- vflush() calls vrele() on the root vnode, right before returning.

All of this allows the unmount to proceed, but this means that most of
afs_FlushVCache() doesn't actually run for the root vcache, and it
means we always log a warning like this on unmount:

    afs_vop_reclaim: afs_FlushVCache failed code 16 [...]

In addition, this means that setting afs_globalVp at the beginning of
afs_unmount() is largely pointless, since it gets set to a vcache
again near the beginning of vflush().

To avoid all of this, stop lying to vflush about how many references
to the root vnode we hold, and just say that we hold 0 references.

Change-Id: Ib434c5fc48e67c3863fcad41279c3d9e0e0b8c2b
Reviewed-on: https://gerrit.openafs.org/13709
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
src/afs/FBSD/osi_vfsops.c