With recent changes to d_invalidate's semantics (it returns void in Linux 3.11,
and always returns success in RHEL 7.4), it has become increasingly clear that
d_invalidate() is not the best function for use in our best-effort
(nondisruptive) attempt to free up vcaches that is afs_ShakeLooseVCaches().
The new d_invalidate() semantics always force the invalidation of a directory
dentry, which contradicts our desire to be nondisruptive, especially when
that directory is being used as the current working directory for a process.
Our call to d_invalidate(), intended to merely probe for whether a dentry
can be discarded without affecting other consumers, instead would cause
processes using that dentry as a CWD to receive ENOENT errors from getcwd().
A previous commit (
c3bbf0b4444db88192eea4580ac9e9ca3de0d286) tried to address
this issue by calling d_prune_aliases() instead of d_invalidate(), but
d_prune_aliases() does not recursively descend into children of the given
dentry while pruning, leaving it an incomplete solution for our use-case.
To address these issues, modify the shakeloose routine TryEvictDentries() to
call shrink_dcache_parent() and maybe __d_drop() for directories, and
d_prune_aliases() for non-directories, instead of d_invalidate(). (Calls to
d_prune_aliases() for directories have already been removed by reverting commit
c3bbf0b4444db88192eea4580ac9e9ca3de0d286.)
Just like d_invalidate(), shrink_dcache_parent() has been around "forever"
(since pre-git v2.6.12). Also like d_invalidate(), it "walks" the parent
dentry's subdirectories and "shrinks" (unhashes) unused dentries. But unlike
d_invalidate(), shrink_dcache_parent() will not unhash an in-use dentry, and
has never changed its signature or semantics.
d_prune_aliases() has also been available "forever", and has also never changed
its signature or semantics. The lack of recursive descent is not an issue for
non-directories, which cannot have such children.
[kaduk@mit.edu: apply review feedback to fix locking and avoid extraneous
changes, and reword commit message]
Change-Id: Icb6138ee5785e0ef82a9b85b1d2651dfd0830043
Reviewed-on: https://gerrit.openafs.org/12830
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
#include "osi_compat.h"
static void
-TryEvictDentries(struct vcache *avc)
+TryEvictDirDentries(struct inode *inode)
{
struct dentry *dentry;
- struct inode *inode = AFSTOV(avc);
#if defined(D_ALIAS_IS_HLIST) && !defined(HLIST_ITERATOR_NO_NODE)
struct hlist_node *p;
#endif
afs_linux_dget(dentry);
afs_d_alias_unlock(inode);
- if (afs_d_invalidate(dentry) == -EBUSY) {
- dput(dentry);
- /* perhaps lock and try to continue? (use cur as head?) */
- goto inuse;
+
+ /*
+ * Once we have dropped the d_alias lock (above), it is no longer safe
+ * to 'continue' our iteration over d_alias because the list may change
+ * out from under us. Therefore, we must either leave the loop, or
+ * restart from the beginning. To avoid looping forever, we must only
+ * restart if we know we've d_drop'd an alias. In all other cases we
+ * must leave the loop.
+ */
+
+ /*
+ * For a long time we used d_invalidate() for this purpose, but
+ * using shrink_dcache_parent() and checking the refcount ourselves is
+ * better, for two reasons: it avoids causing ENOENT issues for the
+ * CWD in linux versions since 3.11, and it avoids dropping Linux
+ * submounts.
+ *
+ * For non-fakestat, AFS mountpoints look like directories and end up here.
+ */
+
+ shrink_dcache_parent(dentry);
+ spin_lock(&dentry->d_lock);
+ if (afs_dentry_count(dentry) > 1) /* still has references */ {
+ if (dentry->d_inode != NULL) /* is not a negative dentry */ {
+ spin_unlock(&dentry->d_lock);
+ dput(dentry);
+ goto inuse;
+ }
}
+ /*
+ * This is either a negative dentry, or a dentry with no references.
+ * Either way, it is okay to unhash it now.
+ * Do so under the d_lock (that is, via __d_drop() instead of d_drop())
+ * to avoid a race with another process picking up a reference.
+ */
+ __d_drop(dentry);
+ spin_unlock(&dentry->d_lock);
+
dput(dentry);
afs_d_alias_lock(inode);
goto restart;
/* First, see if we can evict the inode from the dcache */
if (defersleep && avc != afs_globalVp && VREFCOUNT(avc) > 1
&& avc->opens == 0) {
+ struct inode *ip = AFSTOV(avc);
+
*slept = 1;
AFS_FAST_HOLD(avc);
ReleaseWriteLock(&afs_xvcache);
AFS_GUNLOCK();
- TryEvictDentries(avc);
+ if (S_ISDIR(ip->i_mode))
+ TryEvictDirDentries(ip);
+ else
+ d_prune_aliases(ip);
AFS_GLOCK();
ObtainWriteLock(&afs_xvcache, 733);