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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140529051647.GG18016@ZenIV.linux.org.uk>
Date:	Thu, 29 May 2014 06:16:47 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Miklos Szeredi <mszeredi@...e.cz>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: fs/dcache.c - BUG: soft lockup - CPU#5 stuck for 22s!
 [systemd-udevd:1667]

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@...iv.linux.org.uk> 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@...iv.linux.org.uk>
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@...iv.linux.org.uk>

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/