From: Al Viro <viro@ZenIV.linux.org.uk>
To: Khazhismel Kumykov <khazhy@google.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Miklos Szeredi <mszeredi@redhat.com>
Subject: Re: Hang/soft lockup in d_invalidate with simultaneous calls
Date: Sat, 3 Jun 2017 07:20:09 +0100 [thread overview]
Message-ID: <20170603062009.GI6365@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CACGdZYLjD=g0Ye=L-iVNQ7y5+2_8uw1SLy2et2z70+wBQz4_2w@mail.gmail.com>
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@zeniv.linux.org.uk> 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();
}
}
next prev parent reply other threads:[~2017-06-03 6:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-16 0:05 Hang/soft lockup in d_invalidate with simultaneous calls Khazhismel Kumykov
2017-05-17 21:58 ` Khazhismel Kumykov
2017-05-22 18:18 ` Khazhismel Kumykov
2017-05-25 22:31 ` Khazhismel Kumykov
2017-06-03 1:12 ` Al Viro
2017-06-03 5:22 ` Khazhismel Kumykov
2017-06-03 6:20 ` Al Viro [this message]
2017-06-03 6:47 ` Khazhismel Kumykov
2017-06-12 23:00 ` Khazhismel Kumykov
2017-06-15 10:50 ` Al Viro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170603062009.GI6365@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=khazhy@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.