Re: [PATCH v2] hung_task:add detecting task in D state milliseconds timeout

From: Matthew Wilcox
Date: Sun Jul 05 2020 - 13:16:49 EST


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.