Re: [PATCH] kernel/trace: Stop and wait for kthread on preempt irq module unload

From: Xiao Yang
Date: Wed Apr 29 2020 - 07:48:03 EST


On 2020/4/28 22:15, Joel Fernandes wrote:
I am wondering if it is because in your test, the kthread exits too quickly.
We have these comments in kthread_stop():
* If threadfn() may call do_exit() itself, the caller must ensure
* task_struct can't go away.

Does the below diff on top of the previous patch help?

---8<-----------------------

diff --git a/kernel/trace/preemptirq_delay_test.c b/kernel/trace/preemptirq_delay_test.c
index 1c28ca20e30b6..8051946a18989 100644
--- a/kernel/trace/preemptirq_delay_test.c
+++ b/kernel/trace/preemptirq_delay_test.c
@@ -152,6 +152,8 @@ static int __init preemptirq_delay_init(void)
int retval;

test_task = preemptirq_start_test();
+ get_task_struct(test_task);
+
retval = PTR_ERR_OR_ZERO(test_task);
if (retval != 0)
return retval;
@@ -172,8 +174,10 @@ static void __exit preemptirq_delay_exit(void)
{
kobject_put(preemptirq_delay_kobj);

- if (test_task)
+ if (test_task) {
kthread_stop(test_task);
+ put_task_struct(test_task);
+ }
}

Hi Joel,

Thanks for your additional patch.

First, We have to avoid kbuild error by including <linux/sched/task.h>
---------------------------------------
kernel/trace/preemptirq_delay_test.c: In function âpreemptirq_delay_initâ:
kernel/trace/preemptirq_delay_test.c:155:2: error: implicit declaration of function âget_task_structâ; did you mean âset_task_cpuâ? [-Werror=implicit-function-declaration]
get_task_struct(test_task);
^~~~~~~~~~~~~~~
set_task_cpu
kernel/trace/preemptirq_delay_test.c: In function âpreemptirq_delay_exitâ:
kernel/trace/preemptirq_delay_test.c:179:3: error: implicit declaration of function âput_task_structâ; did you mean âset_task_cpuâ? [-Werror=implicit-function-declaration]
put_task_struct(test_task);
^~~~~~~~~~~~~~~
set_task_cpu
cc1: some warnings being treated as errors
---------------------------------------

Second, I used the following steps to do test and didn't get any warning/panic after applying your additional patchï
---------------------------------------
for i in $(seq 1 100); do modprobe preemptirq_delay_test test_mode=irq delay=500000; rmmod preemptirq_delay_test; done
for i in $(seq 1 100); do modprobe preemptirq_delay_test test_mode=preempt delay=500000; rmmod preemptirq_delay_test; done
---------------------------------------

Thanks,
Xiao Yang