On May 7, 2020 6:05:02 AM EDT, Xiao Yang <yangx...@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...@cn.fujitsu.com>
>
Thanks!
Reviewed-by: Joel Fernandes <j...@joelfernandes.org>
- Joel
>Thanks,
>Xiao Yang
>On 2020/5/6 22:30, Steven Rostedt wrote:
>> From: "Steven Rostedt (VMware)"<rost...@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.2080...@cn.fujitsu.com
>>
>> Cc: sta...@vger.kernel.org
>> Fixes: 793937236d1ee ("lib: Add module for testing preemptoff/irqsoff
>latency tracers")
>> Reported-by: Xiao Yang<yangx...@cn.fujitsu.com>
>> Signed-off-by: Steven Rostedt (VMware)<rost...@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.
