From: Boqun Feng <boqun.feng@gmail.com>
To: Wei Fang <fangwei1@huawei.com>
Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
akpm@linux-foundation.org, jack@suse.com, axboe@kernel.dk,
tj@kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH v2] fs/dcache.c: avoid soft-lockup in dput()
Date: Wed, 22 Jun 2016 14:51:28 +0800 [thread overview]
Message-ID: <20160622065128.GB28443@insomnia> (raw)
In-Reply-To: <1466564475-30417-1-git-send-email-fangwei1@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]
Hi Wei Fang,
On Wed, Jun 22, 2016 at 11:01:15AM +0800, Wei Fang wrote:
> We triggered soft-lockup under stress test which
> open/access/write/close one file concurrently on more than
> five different CPUs:
>
> WARN: soft lockup - CPU#0 stuck for 11s! [who:30631]
> ...
> [<ffffffc0003986f8>] dput+0x100/0x298
> [<ffffffc00038c2dc>] terminate_walk+0x4c/0x60
> [<ffffffc00038f56c>] path_lookupat+0x5cc/0x7a8
> [<ffffffc00038f780>] filename_lookup+0x38/0xf0
> [<ffffffc000391180>] user_path_at_empty+0x78/0xd0
> [<ffffffc0003911f4>] user_path_at+0x1c/0x28
> [<ffffffc00037d4fc>] SyS_faccessat+0xb4/0x230
>
> ->d_lock trylock may failed many times because of concurrently
> operations, and dput() may execute a long time.
>
> Fix this by replacing cpu_relax() with cond_resched().
> dput() used to be sleepable, so make it sleepable again
> should be safe.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Wei Fang <fangwei1@huawei.com>
> ---
> Changes v1->v2:
> - add might_sleep() to annotate that dput() can sleep
>
> fs/dcache.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index d5ecc6e..074fc1c 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -578,7 +578,7 @@ static struct dentry *dentry_kill(struct dentry *dentry)
>
> failed:
> spin_unlock(&dentry->d_lock);
> - cpu_relax();
> + cond_resched();
Is it better to put the cond_resched() in the caller(i.e. dput()), right
before "goto repeat"? Because it's obviously a loop there, which makes
the purpose of cond_resched() more straightforward.
Regards,
Boqun
> return dentry; /* try again with same dentry */
> }
>
> @@ -752,6 +752,8 @@ void dput(struct dentry *dentry)
> return;
>
> repeat:
> + might_sleep();
> +
> rcu_read_lock();
> if (likely(fast_dput(dentry))) {
> rcu_read_unlock();
> --
> 1.7.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
next prev parent reply other threads:[~2016-06-22 6:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-22 3:01 [PATCH v2] fs/dcache.c: avoid soft-lockup in dput() Wei Fang
2016-06-22 6:51 ` Boqun Feng [this message]
2016-07-06 2:36 ` Wei Fang
2016-09-16 7:49 ` Vaishali Thakkar
2016-09-16 12:10 ` Al Viro
2016-09-16 12:50 ` Vaishali Thakkar
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=20160622065128.GB28443@insomnia \
--to=boqun.feng@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=fangwei1@huawei.com \
--cc=jack@suse.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=tj@kernel.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 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.