Re: fs/dcache.c - BUG: soft lockup - CPU#5 stuck for 22s! [systemd-udevd:1667]

[Al Viro]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/