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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).