Re: [PATCH 2/2] deal with deadlock in d_walk()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2015/4/26 18:21, Greg KH wrote:
> On Thu, Feb 05, 2015 at 03:33:41PM +0800, hujianyang wrote:
>> From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxx>
>> Cc: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
>> [hujianyang: Backported to 3.10 refer to the work of Ben Hutchings in 3.2:
>>  - As we only have try_to_ascend() and not d_walk(), apply this
>>    change to all callers of try_to_ascend()
>>  - Adjust context to make __dentry_kill() apply to d_kill()]
>> Signed-off-by: hujianyang <hujianyang@xxxxxxxxxx>
>> ---
>>  fs/dcache.c |  102 ++++++++++++++++++++++++++++++++++++-----------------------
>>  1 files changed, 62 insertions(+), 40 deletions(-)
> 
> Can you provide a version for 3.14-stable as well?
> 
> thanks,
> 
> greg k-h
> 
> .
> 

Hi Greg,

Sorry for my delay.

Here is the patch for 3.14-stable. Since we have 'd_walk()' in 3.14-stable, we
could just apply changes as same as original patch and do not need to apply Ben's
fix, commit 20defcec264c from 3.2.y.

Thanks!
Also thanks for the help from Ben!

Hu



From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxx>
Cc: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
[hujianyang: Backported to 3.14 refer to the work of Ben Hutchings in 3.2:
 - Adjust context to make __dentry_kill() apply to d_kill()]
Signed-off-by: hujianyang <hujianyang@xxxxxxxxxx>
---
 fs/dcache.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index c345f5f..a9231c8 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -435,7 +435,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
 	__releases(parent->d_lock)
 	__releases(dentry->d_inode->i_lock)
 {
-	list_del(&dentry->d_child);
+	__list_del_entry(&dentry->d_child);
 	/*
 	 * Inform d_walk() that we are no longer attached to the
 	 * dentry tree
@@ -1123,33 +1123,31 @@ resume:
 	/*
 	 * 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);

@@ -1159,6 +1157,9 @@ out_unlock:
 	return;

 rename_retry:
+	spin_unlock(&this_parent->d_lock);
+	rcu_read_unlock();
+	BUG_ON(seq & 1);
 	if (!retry)
 		return;
 	seq = 1;
-- 
1.6.0.2


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]