Re: Hang/soft lockup in d_invalidate with simultaneous calls

[Al Viro]

On Fri, Jun 02, 2017 at 10:22:39PM -0700, Khazhismel Kumykov wrote:
> On Fri, Jun 2, 2017 at 6:12 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> > Part of that could be relieved if we turned check_and_drop() into
> > static void check_and_drop(void *_data)
> > {
> >         struct detach_data *data = _data;
> >
> >         if (!data->mountpoint && list_empty(&data->select.dispose))
> >                 __d_drop(data->select.start);
> > }
> 
> So with this change, d_invalidate will drop the starting dentry before
> all it's children are dropped? Would it make sense to just drop it
> right off the bat, and let one task handle shrinking all it's
> children?

We can't - not until we know that there's nothing mounted under it.
The thing is, unlike shrink_dcache_parent() we *can* bugger off as
soon as we'd found no victims, nothing mounted and dentry itself
is unhashed.  We can't do anything in select_collect() (we would've
broken shrink_dcache_parent() that way), but we can do unhashing
in check_and_drop() in "really nothing to do" case and we can return
from d_invalidate() after that.  So how about this:

diff --git a/fs/dcache.c b/fs/dcache.c
index cddf39777835..a9f995f6859e 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1494,7 +1494,7 @@ static void check_and_drop(void *_data)
 {
 	struct detach_data *data = _data;
 
-	if (!data->mountpoint && !data->select.found)
+	if (!data->mountpoint && list_empty(&data->select.dispose))
 		__d_drop(data->select.start);
 }
 
@@ -1536,17 +1536,15 @@ void d_invalidate(struct dentry *dentry)
 
 		d_walk(dentry, &data, detach_and_collect, check_and_drop);
 
-		if (data.select.found)
+		if (!list_empty(&data.select.dispose))
 			shrink_dentry_list(&data.select.dispose);
+		else if (!data.mountpoint)
+			return;
 
 		if (data.mountpoint) {
 			detach_mounts(data.mountpoint);
 			dput(data.mountpoint);
 		}
-
-		if (!data.mountpoint && !data.select.found)
-			break;
-
 		cond_resched();
 	}
 }