lkml.org 
[lkml]   [2020]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] tracing: Wait for preempt irq delay thread to finish
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,
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;
>



\
 
 \ /
  Last update: 2020-05-07 12:05    [W:0.076 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site