Messages in this thread | | | Date | Thu, 07 May 2020 08:29:10 -0400 | Subject | Re: [PATCH] tracing: Wait for preempt irq delay thread to finish | From | joel@joelfern ... |
| |
On May 7, 2020 6:05:02 AM EDT, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote: >Hi Steven, > >Thanks for your further investigation. > >I used the following ways to test your fix patch on my slow vm and >didn't see any issue: >1) Insert and remove preemptirq_delay_test in loops. >2) Insert preemptirq_delay_test, write to >/sys/kernel/preemptirq_delay_test/trigger and remove >preemptirq_delay_test in loops. >3) Ran irqsoff_tracer.tc in loops. > >BTW: For irqsoff_tracer.tc, should we extend code to test the burst >feature and the sysfs trigger? > >Reviewed-by: Xiao Yang <yangx.jy@cn.fujitsu.com> >
Thanks!
Reviewed-by: Joel Fernandes <joel@joelfernandes.org>
- Joel
>Thanks, >Xiao Yang >On 2020/5/6 22:30, Steven Rostedt wrote: >> From: "Steven Rostedt (VMware)"<rostedt@goodmis.org> >> >> Running on a slower machine, it is possible that the preempt delay >kernel >> thread may still be executing if the module was immediately removed >after >> added, and this can cause the kernel to crash as the kernel thread >might be >> executing after its code has been removed. >> >> There's no reason that the caller of the code shouldn't just wait for >the >> delay thread to finish, as the thread can also be created by a >trigger in >> the sysfs code, which also has the same issues. >> >> Link: http://lore.kernel.org/r/5EA2B0C8.2080706@cn.fujitsu.com >> >> Cc: stable@vger.kernel.org >> Fixes: 793937236d1ee ("lib: Add module for testing preemptoff/irqsoff >latency tracers") >> Reported-by: Xiao Yang<yangx.jy@cn.fujitsu.com> >> Signed-off-by: Steven Rostedt (VMware)<rostedt@goodmis.org> >> --- >> kernel/trace/preemptirq_delay_test.c | 30 >++++++++++++++++++++++------ >> 1 file changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/kernel/trace/preemptirq_delay_test.c >b/kernel/trace/preemptirq_delay_test.c >> index 31c0fad4cb9e..c4c86de63cf9 100644 >> --- a/kernel/trace/preemptirq_delay_test.c >> +++ b/kernel/trace/preemptirq_delay_test.c >> @@ -113,22 +113,42 @@ static int preemptirq_delay_run(void *data) >> >> for (i = 0; i< s; i++) >> (testfuncs[i])(i); >> + >> + set_current_state(TASK_INTERRUPTIBLE); >> + while (!kthread_should_stop()) { >> + schedule(); >> + set_current_state(TASK_INTERRUPTIBLE); >> + } >> + >> + __set_current_state(TASK_RUNNING); >> + >> return 0; >> } >> >> -static struct task_struct *preemptirq_start_test(void) >> +static int preemptirq_run_test(void) >> { >> + struct task_struct *task; >> + >> char task_name[50]; >> >> snprintf(task_name, sizeof(task_name), "%s_test", test_mode); >> - return kthread_run(preemptirq_delay_run, NULL, task_name); >> + task = kthread_run(preemptirq_delay_run, NULL, task_name); >> + if (IS_ERR(task)) >> + return PTR_ERR(task); >> + if (task) >> + kthread_stop(task); >> + return 0; >> } >> >> >> static ssize_t trigger_store(struct kobject *kobj, struct >kobj_attribute *attr, >> const char *buf, size_t count) >> { >> - preemptirq_start_test(); >> + ssize_t ret; >> + >> + ret = preemptirq_run_test(); >> + if (ret) >> + return ret; >> return count; >> } >> >> @@ -148,11 +168,9 @@ static struct kobject *preemptirq_delay_kobj; >> >> static int __init preemptirq_delay_init(void) >> { >> - struct task_struct *test_task; >> int retval; >> >> - test_task = preemptirq_start_test(); >> - retval = PTR_ERR_OR_ZERO(test_task); >> + retval = preemptirq_run_test(); >> if (retval != 0) >> return retval; >>
-- Sent from my Android device with K-9 Mail. Please excuse my brevity.
| |