From: Matthew Wilcox <willy@infradead.org>
To: yang che <chey84736@gmail.com>
Cc: mcgrof@kernel.org, keescook@chromium.org, yzaikin@google.com,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2] hung_task:add detecting task in D state milliseconds timeout
Date: Sun, 5 Jul 2020 18:16:33 +0100 [thread overview]
Message-ID: <20200705171633.GU25523@casper.infradead.org> (raw)
In-Reply-To: <1593953332-29404-1-git-send-email-chey84736@gmail.com>
On Sun, Jul 05, 2020 at 08:48:52PM +0800, yang che wrote:
> @@ -16,8 +16,9 @@ extern unsigned int sysctl_hung_task_all_cpu_backtrace;
>
> extern int sysctl_hung_task_check_count;
> extern unsigned int sysctl_hung_task_panic;
> +extern unsigned long sysctl_hung_task_timeout_millisecs;
I would suggest an '_msec' suffix to go along with ...
> @@ -37,6 +37,7 @@ int __read_mostly sysctl_hung_task_check_count = PID_MAX_LIMIT;
> * the RCU grace period. So it needs to be upper-bound.
> */
> #define HUNG_TASK_LOCK_BREAK (HZ / 10)
> +#define SECONDS 1000
We have #define MSEC_PER_SEC 1000L
> @@ -44,9 +45,14 @@ int __read_mostly sysctl_hung_task_check_count = PID_MAX_LIMIT;
> unsigned long __read_mostly sysctl_hung_task_timeout_secs = CONFIG_DEFAULT_HUNG_TASK_TIMEOUT;
>
> /*
> + * Zero means only use sysctl_hung_task_timeout_secs
> + */
> +unsigned long __read_mostly sysctl_hung_task_timeout_millisecs;
Why not:
unsigned long __read_mostly sysctl_hung_task_timeout_msec = \
CONFIG_DEFAULT_HUNG_TASK_TIMEOUT * MSEC_PER_SEC;
> @@ -108,7 +114,8 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
> t->last_switch_time = jiffies;
> return;
> }
> - if (time_is_after_jiffies(t->last_switch_time + timeout * HZ))
> +
> + if (time_is_after_jiffies(t->last_switch_time + (timeout * HZ) / SECONDS))
> return;
We have msecs_to_jiffies() which handles the rounding properly for you.
> - pr_err("INFO: task %s:%d blocked for more than %ld seconds.\n",
> - t->comm, t->pid, (jiffies - t->last_switch_time) / HZ);
> +
> + pr_err("INFO: task %s:%d blocked for more than %ld seconds %ld milliseconds.\n",
I'd use "%ld.%0.3ld seconds" ... or whatever the right format string is.
I have to work it out every time.
> + t->comm, t->pid, (jiffies - t->last_switch_time) / HZ,
> + (jiffies - t->last_switch_time) % HZ * (SECONDS / HZ));
... and again, use jiffies_to_msec() to get the rounding right.
prev parent reply other threads:[~2020-07-05 17:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-05 12:48 [PATCH v2] hung_task:add detecting task in D state milliseconds timeout yang che
2020-07-05 17:16 ` Matthew Wilcox [this message]
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=20200705171633.GU25523@casper.infradead.org \
--to=willy@infradead.org \
--cc=chey84736@gmail.com \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=yzaikin@google.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).