From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754616AbdHVGDr (ORCPT ); Tue, 22 Aug 2017 02:03:47 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:51656 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751384AbdHVGDp (ORCPT ); Tue, 22 Aug 2017 02:03:45 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 6D69E606B7 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=kimran@codeaurora.org Subject: Re: [PATCH] RFC: hung task: Check specific tasks for long uninterruptible sleep state To: "Zhang, Shile (NSB - CN/Hangzhou)" , "mingo@kernel.org" Cc: "imrank140517@gmail.com" , "Luis R. Rodriguez" , Kees Cook , "Peter Zijlstra (Intel)" , Matt Fleming , Andrew Morton , Vegard Nossum , Tetsuo Handa , John Siddle , open list , "open list:PROC SYSCTL" References: <1503311156-16919-1-git-send-email-kimran@codeaurora.org> From: Imran Khan Message-ID: Date: Tue, 22 Aug 2017 11:33:31 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/22/2017 7:24 AM, Zhang, Shile (NSB - CN/Hangzhou) wrote: > Hi, Imran, > Hi Shile, Thanks for getting back on this. > I think a "unmonitored list" is better than "monitor list", because we want khungtaskd can find out the "unexpected" hung task, but not few in a list. > Then, for the fg tasks, which can put it in the "unmonitored list", for the bg tasks, I think we can tweak the timeout to control the duration. > Here fg and bg tasks were just an example and may not be applicable for all environments (non-android). But main intention here is to have some mechanism that would allow to monitor only specific tasks. As we have a sysctl option to enable/disable this mechanism we can at any time fall back to default mechanism of monitoring all tasks. Alternatively this can be done on task priority basis also, where we have a minimum priority and only tasks with priorities higher than this min priority would be monitored and if we keep this min priority to the least priority , all tasks would be monitored like the default case. Please let me know your feedback regarding this approach. Thanks and Regards, Imran > Thanks! > > BR, Shile > > -----Original Message----- > From: Imran Khan [mailto:kimran@codeaurora.org] > Sent: Monday, August 21, 2017 6:26 PM > To: mingo@kernel.org > Cc: imrank140517@gmail.com; Imran Khan ; Luis R. Rodriguez ; Kees Cook ; Peter Zijlstra (Intel) ; Zhang, Shile (NSB - CN/Hangzhou) ; Matt Fleming ; Andrew Morton ; Vegard Nossum ; Tetsuo Handa ; John Siddle ; open list ; open list:PROC SYSCTL > Subject: [PATCH] RFC: hung task: Check specific tasks for long uninterruptible sleep state > > khungtask by default monitors either all tasks or no tasks at all > for long unterruptible sleeps. > For Android like environments this arrangement is not optimal because > on one hand it may be permissible to have some background(bg) task > in uninterruptible sleep state for long duration while on the other > hand it may not be permissible to have some foreground(fg) task like > surfaceflinger in uninterruptible sleep state for long duration. > So it would be good to have some arrangement so that few specified tasks > can be monitored by khungtaskd, on a need basis. > This change introduces a sysctl option, /proc/sys/kernel/ > hung_task_check_selected, to enable monitoring of selected tasks using > khungtask daemon. If this sysctl option is enabled then only the tasks > specified in /proc/hung_task_monitor_list are monitored otherwise all > tasks are monitored, just like the default case. > > Signed-off-by: Imran Khan > --- > include/linux/sched/sysctl.h | 1 + > kernel/hung_task.c | 121 ++++++++++++++++++++++++++++++++++++++++++- > kernel/sysctl.c | 8 +++ > 3 files changed, 128 insertions(+), 2 deletions(-) > > diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h > index 0f5ecd4..05892f1 100644 > --- a/include/linux/sched/sysctl.h > +++ b/include/linux/sched/sysctl.h > @@ -10,6 +10,7 @@ > extern unsigned int sysctl_hung_task_panic; > extern unsigned long sysctl_hung_task_timeout_secs; > extern int sysctl_hung_task_warnings; > +extern int sysctl_hung_task_check_selected; > extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write, > void __user *buffer, > size_t *lenp, loff_t *ppos); > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > index 751593e..49f13fb 100644 > --- a/kernel/hung_task.c > +++ b/kernel/hung_task.c > @@ -16,12 +16,28 @@ > #include > #include > #include > +#include > +#include > +#include > #include > #include > > #include > > /* > + * Hung task that needs monitoring > + */ > +struct hung_task { > + struct list_head list; > + char comm[TASK_COMM_LEN]; > +}; > + > +static struct hung_task *monitor_list; > +int sysctl_hung_task_check_selected; > + > + > + > +/* > * The number of tasks checked: > */ > int __read_mostly sysctl_hung_task_check_count = PID_MAX_LIMIT; > @@ -76,6 +92,92 @@ static int __init hung_task_panic_setup(char *str) > .notifier_call = hung_task_panic, > }; > > +static void hung_task_monitor_setup(void) > +{ > + monitor_list = kmalloc(sizeof(*monitor_list), GFP_KERNEL); > + if (monitor_list) { > + INIT_LIST_HEAD(&monitor_list->list); > + memset(monitor_list->comm, 0, TASK_COMM_LEN); > + } > +} > + > + > +static int hung_task_info_show(struct seq_file *m, void *v) > +{ > + struct hung_task *ht; > + > + ht = list_entry(v, struct hung_task, list); > + seq_puts(m, ht->comm); > + > + return 0; > +} > + > +static void *hung_task_info_start(struct seq_file *m, loff_t *pos) > +{ > + return seq_list_start_head(&monitor_list->list, *pos); > +} > + > +static void *hung_task_info_next(struct seq_file *m, void *v, loff_t *pos) > +{ > + return seq_list_next(v, &monitor_list->list, pos); > +} > + > +static void hung_task_info_stop(struct seq_file *m, void *v) > +{ > +} > + > +const struct seq_operations hung_task_info_op = { > + .start = hung_task_info_start, > + .next = hung_task_info_next, > + .stop = hung_task_info_stop, > + .show = hung_task_info_show > +}; > + > +static int hung_task_info_open(struct inode *inode, struct file *file) > +{ > + return seq_open(file, &hung_task_info_op); > +} > + > +static ssize_t > +hung_task_info_write(struct file *file, const char __user *buf, size_t count, > + loff_t *offs) > +{ > + struct task_struct *g, *t; > + struct hung_task *ht = kmalloc(sizeof(*ht), GFP_KERNEL); > + > + if (!ht) > + return -ENOMEM; > + > + if (copy_from_user(ht->comm, buf, count)) > + return -EFAULT; > + ht->comm[count] = '\0'; > + > + for_each_process_thread(g, t) { > + if (!strncmp(t->comm, ht->comm, strlen(t->comm))) { > + list_add_tail(&ht->list, &monitor_list->list); > + return count; > + } > + } > + > + pr_err("Non-existing task: %s can't be monitored\n", ht->comm); > + return count; > +} > + > +static const struct file_operations hung_task_info_operations = { > + .open = hung_task_info_open, > + .read = seq_read, > + .write = hung_task_info_write, > + .llseek = seq_lseek, > + .release = seq_release, > +}; > + > +static int __init proc_hung_task_info_init(void) > +{ > + proc_create("hung_task_monitor_list", 0644, NULL, > + &hung_task_info_operations); > + return 0; > +} > + > static void check_hung_task(struct task_struct *t, unsigned long timeout) > { > unsigned long switch_count = t->nvcsw + t->nivcsw; > @@ -167,6 +269,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > int max_count = sysctl_hung_task_check_count; > int batch_count = HUNG_TASK_BATCHING; > struct task_struct *g, *t; > + struct hung_task *ht, *tmp; > > /* > * If the system crashed already then all bets are off, > @@ -186,9 +289,21 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > goto unlock; > } > /* use "==" to skip the TASK_KILLABLE tasks waiting on NFS */ > - if (t->state == TASK_UNINTERRUPTIBLE) > - check_hung_task(t, timeout); > + if (t->state == TASK_UNINTERRUPTIBLE) { > + if (sysctl_hung_task_check_selected) { > + list_for_each_entry_safe(ht, tmp, > + &monitor_list->list, > + list) > + if (!strncmp(ht->comm, t->comm, > + strlen(t->comm))) > + /* Task belongs to the selected group */ > + check_hung_task(t, timeout); > + } else { > + check_hung_task(t, timeout); > + } > + } > } > + > unlock: > rcu_read_unlock(); > if (hung_task_show_lock) > @@ -259,6 +374,8 @@ static int watchdog(void *dummy) > static int __init hung_task_init(void) > { > atomic_notifier_chain_register(&panic_notifier_list, &panic_block); > + hung_task_monitor_setup(); > + proc_hung_task_info_init(); > watchdog_task = kthread_run(watchdog, NULL, "khungtaskd"); > > return 0; > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 6648fbb..ab45774 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1096,6 +1096,14 @@ static int sysrq_sysctl_handler(struct ctl_table *table, int write, > .proc_handler = proc_dointvec_minmax, > .extra1 = &neg_one, > }, > + { > + .procname = "hung_task_check_selected", > + .data = &sysctl_hung_task_check_selected, > + .maxlen = sizeof(int), > + .mode = 0644, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &neg_one, > + }, > #endif > #ifdef CONFIG_RT_MUTEXES > { > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation