From: Andrew Morton <akpm@linux-foundation.org>
To: Khazhismel Kumykov <khazhy@google.com>
Cc: nborisov@suse.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
David Rientjes <rientjes@google.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Goldwyn Rodrigues <rgoldwyn@suse.de>,
Jeff Mahoney <jeffm@suse.com>,
Davidlohr Bueso <dave@stgolabs.net>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()
Date: Fri, 13 Apr 2018 14:14:30 -0700 [thread overview]
Message-ID: <20180413141430.2788e2562e3e24bd273fe78b@linux-foundation.org> (raw)
In-Reply-To: <20180413202823.204377-1-khazhy@google.com>
On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov <khazhy@google.com> wrote:
> shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list.
> In this case we may have 0 dentries to dispose, so we will never
> schedule out while waiting for the parallel shrink_dentry_list to
> complete.
>
> Tested that this fixes syzbot reports of stalls in shrink_dcache_parent()
Well I guess the patch is OK as a stopgap, but things seem fairly
messed up in there. shrink_dcache_parent() shouldn't be doing a
busywait, waiting for the concurrent shrink_dentry_list().
Either we should be waiting (sleeping) for the concurrent operation to
complete or we should just bail out of shrink_dcache_parent(), perhaps
with
if (list_empty(&data.dispose))
break;
or similar. Dunno.
That block comment over `struct select_data' is not a good one. "It
returns zero iff...". *What* returns zero? select_collect()? No it
doesn't, it returns an `enum d_walk_ret'. Perhaps the comment is
trying to refer to select_data.found. And the real interpretation of
select_data.found is, umm, hard to describe. "Counts the number of
dentries which are on a shrink list or which were moved to the dispose
list". Why? What's that all about?
This code needs a bit of thought, documentation and perhaps a redo,
I suspect.
next prev parent reply other threads:[~2018-04-13 21:14 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20180413181350.88831-1-khazhy@google.com>
2018-04-13 20:28 ` Khazhismel Kumykov
2018-04-13 21:14 ` Andrew Morton [this message]
2018-04-14 7:00 ` Nikolay Borisov
2018-04-14 8:02 ` Al Viro
2018-04-14 16:36 ` Linus Torvalds
2018-04-14 20:58 ` Al Viro
2018-04-14 21:47 ` Linus Torvalds
2018-04-15 0:51 ` Al Viro
2018-04-15 2:39 ` Al Viro
2018-04-15 14:21 ` Al Viro
2018-04-15 18:34 ` Linus Torvalds
2018-04-15 20:40 ` Al Viro
2018-04-15 21:54 ` Al Viro
2018-04-15 22:34 ` Al Viro
2018-04-16 18:28 ` Khazhismel Kumykov
2018-04-13 21:15 ` David Rientjes
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=20180413141430.2788e2562e3e24bd273fe78b@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=dave@stgolabs.net \
--cc=jeffm@suse.com \
--cc=khazhy@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=rgoldwyn@suse.de \
--cc=rientjes@google.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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
all inboxes | Powered by JetHome®