deal with deadlock in d_walk()

commit ca5358ef75fc69fee5322a38a340f5739d997c10 upstream. ... by not hitting rename_retry for reasons other than rename having happened. In other words, do _not_ restart when finding that between unlocking the child and locking the parent the former got into __dentry_kill(). Skip the killed siblings instead... Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

deal with deadlock in d_walk()
commit ca5358ef75fc69fee5322a38a340f5739d997c10 upstream. ... by not hitting rename_retry for reasons other than rename having happened. In other words, do _not_ restart when finding that between unlocking the child and locking the parent the former got into __dentry_kill(). Skip the killed siblings instead... Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Al Viro · Greg Kroah-Hartman
1 parent 679829c2e5
Showing 1 changed file with 16 additions and 15 deletions Side-by-side Diff
fs/dcache.c
@@ -495,7 +495,7 @@
 	}
 	/* if it was on the hash then remove it */
 	__d_drop(dentry);
-	list_del(&dentry->d_child);
+	__list_del_entry(&dentry->d_child);
 	/*
 	 * Inform d_walk() that we are no longer attached to the
 	 * dentry tree
  
  
  
  
  
  
@@ -1082,33 +1082,31 @@
 	/*
 	 * All done at this level ... ascend and resume the search.
 	 */
+	rcu_read_lock();
+ascend:
 	if (this_parent != parent) {
 		struct dentry *child = this_parent;
 		this_parent = child->d_parent;
  
-		rcu_read_lock();
 		spin_unlock(&child->d_lock);
 		spin_lock(&this_parent->d_lock);
  
-		/*
-		 * might go back up the wrong parent if we have had a rename
-		 * or deletion
-		 */
-		if (this_parent != child->d_parent ||
-			 (child->d_flags & DCACHE_DENTRY_KILLED) ||
-			 need_seqretry(&rename_lock, seq)) {
-			spin_unlock(&this_parent->d_lock);
-			rcu_read_unlock();
+		/* might go back up the wrong parent if we have had a rename. */
+		if (need_seqretry(&rename_lock, seq))
 			goto rename_retry;
+		next = child->d_child.next;
+		while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+			if (next == &this_parent->d_subdirs)
+				goto ascend;
+			child = list_entry(next, struct dentry, d_child);
+			next = next->next;
 		}
 		rcu_read_unlock();
-		next = child->d_child.next;
 		goto resume;
 	}
-	if (need_seqretry(&rename_lock, seq)) {
-		spin_unlock(&this_parent->d_lock);
+	if (need_seqretry(&rename_lock, seq))
 		goto rename_retry;
-	}
+	rcu_read_unlock();
 	if (finish)
 		finish(data);
  
@@ -1118,6 +1116,9 @@
 	return;
  
 rename_retry:
+	spin_unlock(&this_parent->d_lock);
+	rcu_read_unlock();
+	BUG_ON(seq & 1);
 	if (!retry)
 		return;
 	seq = 1;
...	...	@@ -495,7 +495,7 @@
495	495	}
496	496	/* if it was on the hash then remove it */
497	497	__d_drop(dentry);
498		- list_del(&dentry->d_child);
	498	+ __list_del_entry(&dentry->d_child);
499	499	/*
500	500	* Inform d_walk() that we are no longer attached to the
501	501	* dentry tree
502	502
503	503
504	504
505	505
506	506
507	507
...	...	@@ -1082,33 +1082,31 @@
1082	1082	/*
1083	1083	* All done at this level ... ascend and resume the search.
1084	1084	*/
	1085	+ rcu_read_lock();
	1086	+ascend:
1085	1087	if (this_parent != parent) {
1086	1088	struct dentry *child = this_parent;
1087	1089	this_parent = child->d_parent;
1088	1090
1089		- rcu_read_lock();
1090	1091	spin_unlock(&child->d_lock);
1091	1092	spin_lock(&this_parent->d_lock);
1092	1093
1093		- /*
1094		- * might go back up the wrong parent if we have had a rename
1095		- * or deletion
1096		- */
1097		- if (this_parent != child->d_parent \|\|
1098		- (child->d_flags & DCACHE_DENTRY_KILLED) \|\|
1099		- need_seqretry(&rename_lock, seq)) {
1100		- spin_unlock(&this_parent->d_lock);
1101		- rcu_read_unlock();
	1094	+ /* might go back up the wrong parent if we have had a rename. */
	1095	+ if (need_seqretry(&rename_lock, seq))
1102	1096	goto rename_retry;
	1097	+ next = child->d_child.next;
	1098	+ while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
	1099	+ if (next == &this_parent->d_subdirs)
	1100	+ goto ascend;
	1101	+ child = list_entry(next, struct dentry, d_child);
	1102	+ next = next->next;
1103	1103	}
1104	1104	rcu_read_unlock();
1105		- next = child->d_child.next;
1106	1105	goto resume;
1107	1106	}
1108		- if (need_seqretry(&rename_lock, seq)) {
1109		- spin_unlock(&this_parent->d_lock);
	1107	+ if (need_seqretry(&rename_lock, seq))
1110	1108	goto rename_retry;
1111		- }
	1109	+ rcu_read_unlock();
1112	1110	if (finish)
1113	1111	finish(data);
1114	1112
...	...	@@ -1118,6 +1116,9 @@
1118	1116	return;
1119	1117
1120	1118	rename_retry:
	1119	+ spin_unlock(&this_parent->d_lock);
	1120	+ rcu_read_unlock();
	1121	+ BUG_ON(seq & 1);
1121	1122	if (!retry)
1122	1123	return;
1123	1124	seq = 1;