os/kernel/sched: Add structure to save dead task info #6661

ritesh55555 · 2025-02-09T19:07:17Z

This commit implements code to save dead task pid and name value. It creates two list structure, one for pid list and another for name list that is used to store dead task info. This also adds api to get thread name of a dead task whenever required.

os/kernel/sched/sched_addDeadtask.c

abhishek-samsung · 2025-02-10T12:11:12Z

os/kernel/sched/sched_addDeadtask.c

+ * Description:
+ *   This function removes a task's pid and name info from dead list
+ *   if present. This should be called after creating a task.


Wont this create confusion if we are only printing the most recent name of that pid,

at time1, pid1 is task1, it reserved some memory and terminated, but didnt free it.

at time2, pid1 is reassigned to task2, now according to the above logic, when dead threads list is printed, the memory allocated by task1 will be wrongly shown as allocated by task2.

I think that is ok. The max pid count in our system is 65535 (16 bit pid_t). If this is exhausted, then our counter will roll over and start checking from zero. It will allocate a pid of a previously dead task for the new task. If we want to completely avoid the situation that you mentioned, then we need to keep storing all previous task names also, I think it might be an overkill.

@sunghan-chang Please let us know your opinion.

os/kernel/sched/sched_addDeadtask.c

edwakuwaku · 2025-02-10T16:20:52Z

os/kernel/sched/sched_addDeadtask.c

+
+    leave_critical_section(flags);
+
+    return "NA";


"NA" looks too general, maybe @sunghan-chang can suggest a better one.

os/kernel/sched/sched_addDeadtask.c

os/include/tinyara/sched.h

os/kernel/Kconfig

os/kernel/sched/Make.defs

kishore-sn · 2025-02-11T06:41:52Z

os/kernel/sched/sched_addDeadtask.c

+    } else {
+        dead_pid_t *pid_prev;
+        while (pid_head) {
+            if (pid_head->pid == pid) {


Do we really need to check this?
Since we will add when task dies and remove when new task is created, I think this check is an overhead.
If we remove this check, then we can directly insert the new node at the beginning of the list.

kishore-sn · 2025-02-11T06:56:13Z

os/kernel/sched/sched_addDeadtask.c

+ * Description:
+ *   This function removes a task's pid and name info from dead list
+ *   if present. This should be called after creating a task.


I think that is ok. The max pid count in our system is 65535 (16 bit pid_t). If this is exhausted, then our counter will roll over and start checking from zero. It will allocate a pid of a previously dead task for the new task. If we want to completely avoid the situation that you mentioned, then we need to keep storing all previous task names also, I think it might be an overkill.

@sunghan-chang Please let us know your opinion.

os/kernel/sched/sched_deadtask.c

kishore-sn · 2025-02-19T09:53:53Z

os/kernel/sched/sched_deadtask.c

+
+dead_pid_t *allocate_pid_node(int pid)
+{
+    dead_pid_t *node = (dead_pid_t *)kmm_malloc(sizeof(dead_pid_t));


Let us use zalloc so that the node data is set to zero at the time of creation.

os/kernel/sched/sched_deadtask.c

kishore-sn · 2025-02-19T09:55:28Z

os/kernel/sched/sched_deadtask.c

+ *
+ ****************************************************************************/
+
+void sched_addDeadtask(int pid, char *name)


Please follow naming convention. No capital letters.

kishore-sn · 2025-02-19T10:00:18Z

os/kernel/sched/sched_deadtask.c

+     * Note: whenever we create a task with pid no same as a pid no
+     * that is already present in the dead pid list, we always remove 
+     * it from dead pid list. So when we add a dead pid node, we can
+     * directly add it in the head of the dead pid list as there is no copy */


I think this comment is not required here.
Or we can change as below:

NOTE: In order to make sure that same pid is not added twice to the dead task list, we will always call sched_removedeadtask at the time of task or thread creation.

os/kernel/sched/sched_deadtask.c

os/mm/mm_heap/mm_heapinfo_parse_heap.c

kishore-sn · 2025-02-19T10:13:39Z

os/kernel/sched/sched_releasetcb.c

@@ -154,6 +154,10 @@ int sched_releasetcb(FAR struct tcb_s *tcb, uint8_t ttype)
 			}
 #endif

+#ifdef CONFIG_SCHED_SAVE_DEADTASK
+			sched_addDeadtask(tcb->pid, tcb->name);


Let us use LPWORK work queue for adding and removing data on the dead list.

r-prabu · 2025-02-24T04:43:35Z

os/include/tinyara/sched.h

+ *   None
+ *
+ ****************************************************************************/
+


why do we need to expose this API's ? can't this be static to scheduler itself?

Since we will always add and remove tasks from the list using work queue, we can make these apis static.
Let us keep only the sched_getdeadtaskname api here.

r-prabu · 2025-02-24T04:44:59Z

os/include/tinyara/sched.h

+ *
+ ****************************************************************************/
+
+void sched_deadtasklistinit();


take care of coding guidelines for the declaration .
void sched_deadtasklistinit() -> void sched_deadtasklistinit(void)

r-prabu · 2025-02-24T04:47:35Z

os/kernel/sched/sched_deadtask.c

+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,


update the PR with the log where it can display all the dead task info

samsung-singh

this pr add new data structure to store info of dead task name , this change is good to merge .

samsung-singh · 2025-02-21T12:20:42Z

os/kernel/sched/sched_deadtask.c

+
+    /* point to newly created name node */
+    pid_node->name_ptr = name_node;
+    name_node->ref_cnt++;


It is highly unlikely event but ref_cnt++ inside critical section would be safe.

kishore-sn · 2025-02-25T06:53:55Z

os/include/tinyara/sched.h

+ *   None
+ *
+ ****************************************************************************/
+


Since we will always add and remove tasks from the list using work queue, we can make these apis static.
Let us keep only the sched_getdeadtaskname api here.

kishore-sn · 2025-02-25T06:59:20Z

os/kernel/sched/sched_deadtask.c

+    leave_critical_section(flags);
+}
+
+void work_removedeadtask(void *arg)


These apis should be made static.

kishore-sn · 2025-02-25T07:01:15Z

os/kernel/sched/sched_deadtask.c

+ *
+ ****************************************************************************/
+
+void sched_savedeadtaskinfo(pid_t pid, const char *task_name)


You need to declare these apis in the header file.

- This commit implements code to save dead task pid and name value. It creates two list structure, one for pid list and another for name list that is used to store dead task info. This also adds api to get thread name of a dead task whenever required.

This commit adds code to show dead thread names along with dead thread pid, when showing heap info data.

ritesh55555 force-pushed the dead_task branch 2 times, most recently from 0abba02 to 8202c69 Compare February 10, 2025 05:35

abhishek-samsung reviewed Feb 10, 2025

View reviewed changes

edwakuwaku reviewed Feb 10, 2025

View reviewed changes

ritesh55555 force-pushed the dead_task branch 3 times, most recently from 39a932a to 51db4ea Compare February 11, 2025 06:42

kishore-sn reviewed Feb 11, 2025

View reviewed changes

ritesh55555 force-pushed the dead_task branch 3 times, most recently from f382bd6 to 8834581 Compare February 11, 2025 08:01

kishore-sn reviewed Feb 19, 2025

View reviewed changes

ritesh55555 force-pushed the dead_task branch 7 times, most recently from 519599d to f2aae2c Compare February 21, 2025 11:33

r-prabu reviewed Feb 24, 2025

View reviewed changes

samsung-singh approved these changes Feb 24, 2025

View reviewed changes

ritesh55555 force-pushed the dead_task branch 2 times, most recently from 25001ca to b465587 Compare February 24, 2025 10:52

kishore-sn reviewed Feb 25, 2025

View reviewed changes

ritesh55555 added 2 commits February 25, 2025 14:20

os/mm: show dead thread name while printing dead thread list

53eef80

This commit adds code to show dead thread names along with dead thread pid, when showing heap info data.

ritesh55555 force-pushed the dead_task branch from b465587 to 53eef80 Compare February 25, 2025 08:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os/kernel/sched: Add structure to save dead task info #6661

os/kernel/sched: Add structure to save dead task info #6661

ritesh55555 commented Feb 9, 2025

abhishek-samsung Feb 10, 2025

kishore-sn Feb 11, 2025

edwakuwaku Feb 10, 2025

kishore-sn Feb 11, 2025

kishore-sn Feb 11, 2025

kishore-sn Feb 19, 2025

kishore-sn Feb 19, 2025

kishore-sn Feb 19, 2025

kishore-sn Feb 19, 2025

r-prabu Feb 24, 2025

kishore-sn Feb 25, 2025

r-prabu Feb 24, 2025

r-prabu Feb 24, 2025

samsung-singh left a comment

samsung-singh Feb 21, 2025

kishore-sn Feb 25, 2025

kishore-sn Feb 25, 2025

kishore-sn Feb 25, 2025

os/kernel/sched: Add structure to save dead task info #6661

Are you sure you want to change the base?

os/kernel/sched: Add structure to save dead task info #6661

Conversation

ritesh55555 commented Feb 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samsung-singh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment