On Thu, May 29, 2014 at 06:16:47AM +0100, Al Viro wrote:
> 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.
Except that it's from the wrong branch ;-/ Correct one follows:
commit fb860957449c07c6f1a1dd911e83a635b6f11f21
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Thu May 29 00:37:49 2014 -0400
deal with "put the parent" side of shrink_dentry_list()
diff --git a/fs/dcache.c b/fs/dcache.c
index 6c2a92e..628a791 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -793,30 +793,34 @@ 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;
+ 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;
+ if (!IS_ROOT(dentry)) {
+ parent = dentry->d_parent;
+ 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;
- dentry = list_entry(list->prev, struct dentry, d_lru);
+ struct dentry *dentry = list_entry(list->prev, struct dentry, d_lru);
+ struct dentry *parent;
- 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);
- }
- }
+ parent = lock_parent(dentry);
/*
* The dispose list is isolated and dentries are not accounted
@@ -864,8 +868,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
