From c8f8feb7f8bf103088c9a4139158ab91236cce86 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 29 Nov 2017 14:56:45 -0800 Subject: [PATCH] scoutfs: invalidate dentries as locks are dropped Today we use unconditional dentry revalidation to provide directory entry consistency. Any time the vfs tries to use a cached dentry we tell it to drop it and perform a lookup. This hits our item cache which is kept consistent by the locks. This would just be a waste of cpu if it weren't for how heavy weight the vfs revalidation->lookup path is here. It doesn't just invalidate the entry it uses shrink_dcache_parent() to drop all the cached entries in the subtree rooted at the cached entry. We saw 22 second long cpu livelocks in this shrink_dcache_parent() when creating and archiving empty files. Instead lets let the vfs use dcache entries. We only invalidate them as we're dropping the lock that covers them. (Today coarse inode locks cover all the entries in batches of inodes.) We can use d_drop() to remove entries from the cache to stop them from satisfying lookup without trying to free all the dentries under them. Signed-off-by: Zach Brown --- kmod/src/dir.c | 8 -------- kmod/src/lock.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/kmod/src/dir.c b/kmod/src/dir.c index 3aa361d9..1c9a8b8c 100644 --- a/kmod/src/dir.c +++ b/kmod/src/dir.c @@ -120,16 +120,8 @@ static void scoutfs_d_release(struct dentry *dentry) } } -static int scoutfs_d_revalidate(struct dentry *dentry, unsigned int flags) -{ - if (flags & LOOKUP_RCU) - return -ECHILD; - return 0;/* Always revalidate for now */ -} - static const struct dentry_operations scoutfs_dentry_ops = { .d_release = scoutfs_d_release, - .d_revalidate = scoutfs_d_revalidate, }; static int alloc_dentry_info(struct dentry *dentry) diff --git a/kmod/src/lock.c b/kmod/src/lock.c index 2ea01f3b..ce53a440 100644 --- a/kmod/src/lock.c +++ b/kmod/src/lock.c @@ -163,9 +163,26 @@ static int put_task_ref(struct scoutfs_lock *lock, struct task_ref *ref) return 1; } +/* + * invalidate cached data associated with an inode whose lock is going + * away. + * + * Our inode granular locks mean that we have to invalidate all the + * child dentries of a dir so that they can't satisfy lookup after we + * re-acquire the lock. We're invalidating the lock so there can't be + * active users that could modify the entries in the dcache (lookup, + * create, rename, unlink). We have to make it through all the child + * entries and remove them from the hash so that lookup can't find them. + * They can still be disconnected in the cache or used as working + * directories. A directory can have an enormous number of children so + * we try to be break the lock if needed. + */ static void invalidate_inode(struct super_block *sb, u64 ino) { struct inode *inode; + struct dentry *parent; + struct dentry *child; + struct dentry *saved; inode = scoutfs_ilookup(sb, ino); if (!inode) @@ -174,6 +191,42 @@ static void invalidate_inode(struct super_block *sb, u64 ino) if (S_ISREG(inode->i_mode)) truncate_inode_pages(inode->i_mapping, 0); + if (S_ISDIR(inode->i_mode) && (parent = d_find_alias(inode))) { + saved = NULL; +restart: + spin_lock(&parent->d_lock); + if (saved) { + if (saved->d_parent != parent) + child = NULL; + else + child = saved; + dput(saved); + } else { + child = NULL; + } + + if (child == NULL) + child = list_entry(parent->d_subdirs.next, + struct dentry, d_u.d_child); + + list_for_each_entry_from(child, &parent->d_subdirs,d_u.d_child){ + if (spin_needbreak(&parent->d_lock) || need_resched()) { + saved = child; + dget(saved); + spin_unlock(&parent->d_lock); + cond_resched(); + goto restart; + } + + spin_lock_nested(&child->d_lock, DENTRY_D_LOCK_NESTED); + __d_drop(child); + spin_unlock(&child->d_lock); + } + spin_unlock(&parent->d_lock); + + dput(parent); + } + iput(inode); }