Re: [PATCH] tracing: Wait for preempt irq delay thread to finish

[joel]

On May 7, 2020 6:05:02 AM EDT, Xiao Yang <yangx.jy@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
>

Thanks!

Reviewed-by: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx>

- Joel


>Thanks,
>Xiao Yang
>On 2020/5/6 22:30, Steven Rostedt wrote:
>> From: "Steven Rostedt (VMware)"<rostedt@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxx
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 793937236d1ee ("lib: Add module for testing preemptoff/irqsoff
>latency tracers")
>> Reported-by: Xiao Yang<yangx.jy@xxxxxxxxxxxxxx>
>> Signed-off-by: Steven Rostedt (VMware)<rostedt@xxxxxxxxxxx>
>> ---
>>   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.