Remove config dependency to fix ps command issue #6634

neel-samsung · 2025-01-16T13:11:46Z

No description provided.

jeongarmy · 2025-01-17T05:46:06Z

os/kernel/sched/sched_getcpu.c

+
+int sched_getcpucount(void)
+{
+  return CONFIG_SMP_NCPUS;


What happens if this value is not present?
I think we need to define values for when config is not existing.

like

#if defined(CONFIG_SMP_NCPUS) return CONFIG_SMP_NCPUS; #else return 0

or

In the file, #if defined(CONFIG_SMP_NCPUS) #define NCPUS CONFIG_SMP_NCPUS #else #define NCPUS 0 ... in sched_getcpucount() return NCPUS;

We are already defining the config if it is not present.

TizenRT/os/tools/mkconfig.c

Lines 302 to 304 in e28022b

printf("#ifndef CONFIG_SMP_NCPUS\n");

printf("# define CONFIG_SMP_NCPUS 1\n");

printf("#endif\n\n");

jeongarmy · 2025-01-17T05:53:05Z

apps/system/utils/utils_cpuload.c

@@ -179,32 +177,24 @@ static void cpuload_print_pid_value(char *buf, void *arg)
 	} else {
 #ifdef CONFIG_SCHED_MULTI_CPULOAD
 		for (i = PROC_STAT_CPULOAD_SHORT; i <= PROC_STAT_CPULOAD_LONG; i++) {
-#ifdef CONFIG_SMP
-			char * avgload[CONFIG_SMP_NCPUS + 1];
+			char * avgload[ncpus + 1];


ncpus is not static value. you define array with not defined value.. Is it ok? I think it makes warning or error.

I think, this also shouldn't create issue as syscall will return a constant value based on CONFIG_SMP_NCPUS

Actually, we are not supposed to delare array using a variable as the array size.
In this case, it might not cause issue because the array is local to the for loop and the ncpus variable is outside of the loop. However, pls check again to make sure you dont get any warning or error during build.

I have made sure that it doesn't give any warning or failure.

kishore-sn · 2025-01-20T09:39:04Z

Please modify commit descriptions. This work is for ps and cpuload commands.
In app side commit, pls add the output of each of these commands for smp and no smp case.

sunghan-chang · 2025-01-17T10:43:10Z

os/kernel/sched/sched_getcpu.c

+
+int sched_getcpucount(void)
+{
+  return CONFIG_SMP_NCPUS;


Coding rule violation, should use a tab instead of spaces

sunghan-chang · 2025-01-21T06:02:27Z

os/kernel/init/os_start.c

@@ -296,6 +296,10 @@ const struct tasklist_s g_tasklisttable[NUM_TASK_STATES] =
    &g_readytorun,
    TLIST_ATTR_PRIORITIZED | TLIST_ATTR_RUNNABLE
  },
+  {                                              /* TSTATE_TASK_ASSIGNED */


Coding rule violations. Same as other lines

To split different topic of change, in this commit, please modify lines you added only. And please add one more commit to modify other lines violations. (or new pr)

kishore-sn · 2025-01-22T07:13:24Z

Please change commit title remove config dependency to fix ps command issue >>> Remove ssytem commands dependency on CONFIG_SMPxxx

Due to config dependency, system commands don't give correct output. So, this patch fix the issue by removing config dependency for CONFIG_SMP and CONFIG_SMP_NCPUS. It fixes the ps command issue completely but it doesn't completely resolve cpuload command issue. ps output smp disabled: PID | PRIO | FLAG | TYPE | NP | STATUS | CPU | NAME ------|------|------|---------|----|----------|-----|---- 0 | 0 | FIFO | KTHREAD | N | READY | 0 | Idle Task 1 | 201 | RR | KTHREAD | | WAITSIG | 0 | hpwork 2 | 50 | RR | KTHREAD | | WAITSIG | 0 | lpwork 4 | 200 | RR | KTHREAD | | MQNEMPTY | 0 | km4_log_task 5 | 107 | RR | KTHREAD | | WAITSEM | 0 | inic_msg_q_task 6 | 104 | RR | KTHREAD | | WAITSEM | 0 | inic_host_rx_tasklet 7 | 103 | RR | KTHREAD | | WAITSEM | 0 | inic_ipc_api_host_task 9 | 105 | RR | KTHREAD | | WAITSEM | 0 | LWIP_TCP/IP 10 | 100 | RR | KTHREAD | | WAITSEM | 0 | netmgr_event_handler 11 | 200 | RR | KTHREAD | | MQNEMPTY | 0 | log_dump 12 | 203 | RR | KTHREAD | | MQNEMPTY | 0 | binary_manager 15 | 180 | RR | TASK | | WAITSIG | 0 | app1 16 | 100 | RR | TASK | | WAITSIG | 0 | uwork 17 | 125 | RR | TASK | | RUNNING | 0 | tash 18 | 100 | RR | TASK | | WAITSEM | 0 | wifi msg handler 19 | 100 | RR | TASK | | WAITSEM | 0 | ble msg handler ps output smp enabled: PID | PRIO | FLAG | TYPE | NP | STATUS | CPU | NAME ------|------|------|---------|----|----------|-----|---- 0 | 0 | FIFO | KTHREAD | N | ASSIGNED | 0 | CPU0 IDLE 1 | 0 | FIFO | KTHREAD | N | RUNNING | 1 | CPU1 IDLE 2 | 201 | FIFO | KTHREAD | | WAITSIG | 0 | hpwork 3 | 50 | FIFO | KTHREAD | | WAITSIG | 0 | lpwork 5 | 200 | FIFO | KTHREAD | | MQNEMPTY | 0 | km4_log_task 6 | 107 | FIFO | KTHREAD | | WAITSEM | 0 | inic_msg_q_task 7 | 104 | FIFO | KTHREAD | | WAITSEM | 0 | inic_host_rx_tasklet 8 | 103 | FIFO | KTHREAD | | WAITSEM | 0 | inic_ipc_api_host_task 10 | 105 | FIFO | KTHREAD | | WAITSEM | 0 | LWIP_TCP/IP 11 | 100 | FIFO | KTHREAD | | WAITSEM | 0 | netmgr_event_handler 12 | 200 | FIFO | KTHREAD | | MQNEMPTY | 0 | log_dump 13 | 203 | FIFO | KTHREAD | | MQNEMPTY | 0 | binary_manager 16 | 180 | FIFO | TASK | | WAITSIG | 0 | app1 17 | 100 | FIFO | TASK | | WAITSIG | 0 | uwork 18 | 125 | FIFO | TASK | | RUNNING | 0 | tash 19 | 100 | FIFO | TASK | | WAITSEM | 0 | wifi msg handler 20 | 100 | FIFO | TASK | | WAITSEM | 0 | ble msg handler Signed-off-by: neel-samsung <[email protected]>

neel-samsung · 2025-01-23T05:18:02Z

Please change commit title remove config dependency to fix ps command issue >>> Remove ssytem commands dependency on CONFIG_SMPxxx

Updated

kishore-sn · 2025-02-05T05:53:42Z

@sunghan-chang
Please let me know if I can merge this.

sunghan-chang · 2025-02-05T09:31:29Z

@kishore-sn Let's merge.
And let's apply this to syscall as well. I want to remove all conditionals in the syscall.

Due to config dependency, system commands don't give correct output. So, this patch fix the ps command issue by removing config dependency for CONFIG_SMP and CONFIG_SMP_NCPUS between Kernel and application. We handle it by introducing the syscall for obtaining the number of CPUs while runtime. Signed-off-by: neel-samsung <[email protected]>

This patch fixes the indentation from 2 space indent to tab indent. Signed-off-by: neel-samsung <[email protected]>

sunghan-chang · 2025-02-06T02:33:13Z

@kishore-sn Let's merge.
And let's apply this to syscall as well. I want to remove all conditionals in the syscall.

What I mean above is removing all conditionals (not smp only) with new PR.
This is not related with this PR.

sunghan-chang · 2025-02-06T02:33:48Z

@kishore-sn Let's merge this PR.

neel-samsung force-pushed the remove_config_dep branch from 65b699c to c1afe15 Compare January 16, 2025 18:39

jeongarmy reviewed Jan 17, 2025

View reviewed changes

sunghan-chang reviewed Jan 21, 2025

View reviewed changes

neel-samsung force-pushed the remove_config_dep branch from c1afe15 to 783f8be Compare January 21, 2025 10:53

neel-samsung force-pushed the remove_config_dep branch from 783f8be to 8728416 Compare January 23, 2025 05:17

kishore-sn previously approved these changes Feb 5, 2025

View reviewed changes

kishore-sn requested a review from sunghan-chang February 5, 2025 05:53

sunghan-chang previously approved these changes Feb 5, 2025

View reviewed changes

neel-samsung added 2 commits February 5, 2025 11:55

os/kernel: fix improper indentation issue

b29bcbd

This patch fixes the indentation from 2 space indent to tab indent. Signed-off-by: neel-samsung <[email protected]>

neel-samsung dismissed stale reviews from sunghan-chang and kishore-sn via b29bcbd February 5, 2025 12:06

neel-samsung force-pushed the remove_config_dep branch from 8728416 to b29bcbd Compare February 5, 2025 12:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove config dependency to fix ps command issue #6634

Remove config dependency to fix ps command issue #6634

neel-samsung commented Jan 16, 2025

jeongarmy Jan 17, 2025

jeongarmy Jan 17, 2025

neel-samsung Jan 17, 2025

jeongarmy Jan 17, 2025 •

edited

Loading

neel-samsung Jan 17, 2025

kishore-sn Jan 20, 2025

neel-samsung Jan 21, 2025

kishore-sn commented Jan 20, 2025

sunghan-chang Jan 17, 2025

sunghan-chang Jan 21, 2025

sunghan-chang Jan 21, 2025

kishore-sn commented Jan 22, 2025 •

edited

Loading

neel-samsung commented Jan 23, 2025

kishore-sn commented Feb 5, 2025

sunghan-chang commented Feb 5, 2025

sunghan-chang commented Feb 6, 2025 •

edited

Loading

sunghan-chang commented Feb 6, 2025

	printf("#ifndef CONFIG_SMP_NCPUS\n");
	printf("# define CONFIG_SMP_NCPUS 1\n");
	printf("#endif\n\n");

Remove config dependency to fix ps command issue #6634

Are you sure you want to change the base?

Remove config dependency to fix ps command issue #6634

Conversation

neel-samsung commented Jan 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeongarmy Jan 17, 2025 • edited Loading

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

kishore-sn commented Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kishore-sn commented Jan 22, 2025 • edited Loading

neel-samsung commented Jan 23, 2025

kishore-sn commented Feb 5, 2025

sunghan-chang commented Feb 5, 2025

sunghan-chang commented Feb 6, 2025 • edited Loading

sunghan-chang commented Feb 6, 2025

jeongarmy Jan 17, 2025 •

edited

Loading

kishore-sn commented Jan 22, 2025 •

edited

Loading

sunghan-chang commented Feb 6, 2025 •

edited

Loading