zlogger: improve zlog_task 44/297844/2
authorMateusz Majewski <m.majewski2@samsung.com>
Fri, 25 Aug 2023 10:11:10 +0000 (12:11 +0200)
committerMateusz Majewski <m.majewski2@samsung.com>
Fri, 25 Aug 2023 11:08:02 +0000 (13:08 +0200)
Currently, there is some attempt at handling completion, but it does not
work correctly; unloading the module results in an immediate kernel
oops. Additionally, the module attempts to track whether the task is
running, but this is not necessary due to how modules work in Linux. We
rewrite these part to use standard kernel APIs in a more direct way.

Change-Id: I92b815660e81d47e056a436a1d73bdfdde60bce2
Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
kernel/zlogger/zlogger.c

index 599d1b1..427ebf6 100644 (file)
@@ -121,12 +121,10 @@ static struct queue g_free_q;
 static uint64_t g_start_time;
 static uint64_t g_hwc_offset;
 
-static int g_task_on;
+static struct task_struct *g_task;
 
 static uint32_t g_err_count;
 
-static struct completion g_completion;
-
 static int g_zlog_enable = 1;
 module_param_named(zlog_enable, g_zlog_enable, int, 0644);
 
@@ -257,7 +255,7 @@ static int zlog_task(void *user_data)
        int tmp_bkt;
        int blk;
 
-       do {
+       while (!kthread_should_stop()) {
                hash_for_each_safe(g_thread_table->data, tmp_bkt, tmp_iter, ptr, next) {
                        blk = ptr->blk;
                        // TODO: g_start_time should be under some kind of mutex.
@@ -272,38 +270,10 @@ static int zlog_task(void *user_data)
                        }
                }
 
-               // TODO: This makes no sense.
-               wait_for_completion_interruptible_timeout(&g_completion, msecs_to_jiffies(MS_PER_SEC * 5));
-       } while (1);
-
-       g_task_on = 0;
-       do_exit(0);
-
-       return 0;
-}
-
-static void run_task(void)
-{
-       struct task_struct *task;
-
-       if (!mutex_trylock(&g_task_mutex))
-               return;
-
-       if (g_task_on) {
-               // TODO: this mutex is either incorrect or nonsensical,
-               // since the other task will not end (it is an infinite loop after all).
-               mutex_unlock(&g_task_mutex);
-               complete(&g_completion);
-               return;
+               schedule_timeout_interruptible(msecs_to_jiffies(MS_PER_SEC * 5));
        }
-       g_task_on = 1;
-       mutex_unlock(&g_task_mutex);
 
-       task = kthread_run(zlog_task, NULL, "zlog_task");
-       if (IS_ERR(task)) {
-               pr_err("Failed to run zlog_task\n");
-               g_task_on = 0;
-       }
+       return 0;
 }
 
 static struct thread_table_field *alloc_block_for_thread(bool is_stdout)
@@ -953,8 +923,6 @@ static int zlogger_init(void)
        mutex_init(&g_block_mutex);
        mutex_init(&g_task_mutex);
 
-       init_completion(&g_completion);
-
        queue_init(&g_free_q, "free", ZLOGGER_BLOCK_COUNT);
        for (i = 1; i <= ZLOGGER_BLOCK_COUNT; i++)
                queue_push(&g_free_q, i);
@@ -962,7 +930,11 @@ static int zlogger_init(void)
 #ifdef CONFIG_TRACE_CLOCK
        g_hwc_offset = trace_clock_local() - ktime_get_ns();
 #endif
-       run_task();
+       g_task = kthread_run(zlog_task, NULL, "zlog_task");
+       if (IS_ERR(g_task)) {
+               pr_err("Failed to run zlog_task\n");
+               goto out_free_g_thread_table_g_shm_ptr;
+       }
 
        zlogger_device.minor = MISC_DYNAMIC_MINOR;
        zlogger_device.name = ZLOGGER_DEVICE_NAME;
@@ -1000,9 +972,7 @@ out_free_zlogger_device:
        misc_deregister(&zlogger_device);
 
 out_free_zlog_task:
-       /* TODO: actually destroy this task.
-        * We don't do so yet, even in zlogger_exit (see TODO there).
-        */
+       kthread_stop(g_task);
 
 out_free_g_thread_table_g_shm_ptr:
        for (i = 0; i < g_shm_ptr_i; ++i) {
@@ -1026,7 +996,7 @@ static void zlogger_exit(void)
        misc_deregister(&zlogger_dump_device);
        misc_deregister(&zlogger_device);
 
-       // TODO: What about the task that is running in the background?
+       kthread_stop(g_task);
 
        queue_deinit(&g_free_q);
        hash_for_each_safe(g_thread_table->data, tmp_bkt, tmp_iter, ptr, next) {