On Wed, May 28, 2014 at 09:21:00PM -0700, Linus Torvalds wrote:
> On Wed, May 28, 2014 at 8:11 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Here's an updated patch, hopefully slightly less vomit-inducing.
>
> Hmm. Less vomit-inducing, except for this part:
> Ugh, that just *screams* for a helper function. Something like
>
> parent = get_parent_and_lock(dentry);
>
> or whatever, with that trylock/renamelock dance separated out. The
> rule would be that it would lock the "dentry", and return the
> (similarly locked) parent. Or NULL for a root dentry, of course.
>
> Please?
Already done in my current tree; see below for the next commit in there...
I can separate that helper from the rest (dealing with the same livelock
for dentry_kill(dentry, 1) caller in shrink_dentry_list()). All of that
is very much subject to reordering, resplitting, writing more decent commit
messages, etc.
commit 67e2554f6ca62e20363139f6f2968063410f0f3b
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Wed May 28 23:53:36 2014 -0400
deal with "put the parent" side of shrink_dentry_list()
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
diff --git a/fs/dcache.c b/fs/dcache.c
index 6c2a92e..7434169 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -793,30 +793,31 @@ restart:
}
EXPORT_SYMBOL(d_prune_aliases);
-static void shrink_dentry_list(struct list_head *list)
+static inline struct dentry *lock_parent(struct dentry *dentry)
{
- struct dentry *dentry, *parent;
-
- while (!list_empty(list)) {
- struct inode *inode;
- dentry = list_entry(list->prev, struct dentry, d_lru);
-
+ struct dentry *parent = dentry->d_parent;
+ if (IS_ROOT(dentry))
+ return NULL;
+ if (unlikely(!spin_trylock(&parent->d_lock))) {
+ spin_unlock(&dentry->d_lock);
+ read_seqlock_excl(&rename_lock);
parent = NULL;
- spin_lock(&dentry->d_lock);
if (!IS_ROOT(dentry)) {
parent = dentry->d_parent;
- if (unlikely(!spin_trylock(&parent->d_lock))) {
- spin_unlock(&dentry->d_lock);
- parent = NULL;
- read_seqlock_excl(&rename_lock);
- if (!IS_ROOT(dentry)) {
- parent = dentry->d_parent;
- spin_lock(&parent->d_lock);
- }
- read_sequnlock_excl(&rename_lock);
- spin_lock(&dentry->d_lock);
- }
+ spin_lock(&parent->d_lock);
}
+ read_sequnlock_excl(&rename_lock);
+ spin_lock(&dentry->d_lock);
+ }
+ return parent;
+}
+
+static void shrink_dentry_list(struct list_head *list)
+{
+ while (!list_empty(list)) {
+ struct inode *inode;
+ struct dentry *dentry = list_entry(list->prev, struct dentry, d_lru);
+ struct dentry *parent = lock_parent(dentry);
/*
* The dispose list is isolated and dentries are not accounted
@@ -864,8 +865,26 @@ static void shrink_dentry_list(struct list_head *list)
* fragmentation.
*/
dentry = parent;
- while (dentry && !lockref_put_or_lock(&dentry->d_lockref))
- dentry = dentry_kill(dentry);
+ while (dentry && !lockref_put_or_lock(&dentry->d_lockref)) {
+ parent = lock_parent(dentry);
+ if (dentry->d_lockref.count != 1) {
+ dentry->d_lockref.count--;
+ spin_unlock(&dentry->d_lock);
+ if (parent)
+ spin_unlock(&parent->d_lock);
+ break;
+ }
+ inode = dentry->d_inode; /* can't be NULL */
+ if (unlikely(!spin_trylock(&inode->i_lock))) {
+ spin_unlock(&dentry->d_lock);
+ if (parent)
+ spin_unlock(&parent->d_lock);
+ cpu_relax();
+ continue;
+ }
+ __dentry_kill(dentry);
+ dentry = parent;
+ }
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
