Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove config dependency to fix ps command issue #6634

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 16 additions & 28 deletions apps/system/utils/utils_cpuload.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ static void cpuload_stop(void)

static bool has_cpuload(char *load_info)
{
#ifdef CONFIG_SMP
int len = strlen(load_info);
int ncpus = sched_getcpucount();

if (cpu == CONFIG_SMP_NCPUS) {
if (cpu == ncpus) {
/* This is the case when user has not specified cpu idx.
* In this case we check the avg value. If avg is zero, then
* all cpu load values will be zero and hence we return false.
Expand All @@ -133,14 +133,11 @@ static bool has_cpuload(char *load_info)
return (strncmp(load_info, "0.0", 3) != 0);
} else {
char *str = load_info;
for (int i = 1; i < CONFIG_SMP_NCPUS && i <= cpu; i++) {
for (int i = 1; i < ncpus && i <= cpu; i++) {
str = strchr(str, '-') + 1;
}
return (strncmp(str, "0.0", 3) != 0);
}
#else
return (strncmp(load_info, "0.0", 3) != 0);
#endif
}

static void cpuload_print_pid_value(char *buf, void *arg)
Expand All @@ -150,6 +147,7 @@ static void cpuload_print_pid_value(char *buf, void *arg)
int pid_hash;
double load_ratio;
stat_data stat_info[PROC_STAT_MAX];
int ncpus = sched_getcpucount();

stat_info[0] = buf;

Expand Down Expand Up @@ -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];
Copy link
Contributor

@jeongarmy jeongarmy Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

avgload[0] = stat_info[i];
for (int j = 0; j < CONFIG_SMP_NCPUS; j++) {
for (int j = 0; j < ncpus; j++) {
avgload[j] = strtok_r(avgload[j], "-", &avgload[j + 1]);
if (cpu >= CONFIG_SMP_NCPUS || cpu == j) {
if (cpu >= ncpus || cpu == j) {
printf(" %5s |", avgload[j]);
}
}
#else
printf(" %5s |", stat_info[i]);
#endif
}
#else
#ifdef CONFIG_SMP
char * avgload[CONFIG_SMP_NCPUS + 1];
char * avgload[ncpus + 1];
avgload[0] = stat_info[PROC_STAT_CPULOAD];
for (i = 0; i < CONFIG_SMP_NCPUS; i++) {
for (i = 0; i < ncpus; i++) {
avgload[i] = strtok_r(avgload[i], "-", &avgload[i + 1]);
if (cpu >= CONFIG_SMP_NCPUS || cpu == i) {
if (cpu >= ncpus || cpu == i) {
printf(" %5s |", avgload[i]);
}
}
#else
printf(" %5s |", stat_info[PROC_STAT_CPULOAD]);
#endif
#endif
}
#if (CONFIG_TASK_NAME_SIZE > 0)
Expand Down Expand Up @@ -232,6 +222,7 @@ static int cpuload_read_proc(FAR struct dirent *entryp, FAR void *arg)

static void cpuload_print_normal(void)
{
int ncpus = sched_getcpucount();
/* Print titles */
printf("\n--------------------------------------------------\n");
printf("Non-Zero CPU utilization trend (updated every %ds)\n", cpuload_interval);
Expand All @@ -244,15 +235,11 @@ static void cpuload_print_normal(void)
for (int i = PROC_STAT_CPULOAD_SHORT; i <= PROC_STAT_CPULOAD_LONG; i++)
#endif
{
#ifdef CONFIG_SMP
for (int j = 0; j < CONFIG_SMP_NCPUS; j++) {
if (cpu >= CONFIG_SMP_NCPUS || cpu == j) {
for (int j = 0; j < ncpus; j++) {
if (cpu >= ncpus || cpu == j) {
printf(" CPU%d |", j);
}
}
#else
printf(" CPU |");
#endif
printf(" Task Name ");
}

Expand Down Expand Up @@ -443,6 +430,7 @@ int utils_cpuload(int argc, char **args)
int opt;
int ret;
long value;
int ncpus = sched_getcpucount();

if (argc >= 2) {
if (!strncmp(args[1], "--help", strlen("--help") + 1)) {
Expand All @@ -464,7 +452,7 @@ int utils_cpuload(int argc, char **args)
cpuload_mode = CPULOAD_NORMAL;
cpuload_interval = CONFIG_CPULOADMONITOR_INTERVAL;
cpuload_count = CPULOADMON_RUNNING_FOREVER;
cpu = CONFIG_SMP_NCPUS;
cpu = ncpus;

if (argc > 1) {
/*
Expand Down Expand Up @@ -508,7 +496,7 @@ int utils_cpuload(int argc, char **args)
case 'c':
/* set count of iterations */
value = atoi(optarg);
if (value < 0 || value >= CONFIG_SMP_NCPUS) {
if (value < 0 || value >= ncpus) {
printf("Invalid input for -c option");
goto show_usage;
}
Expand Down
2 changes: 0 additions & 2 deletions apps/system/utils/utils_proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ enum proc_stat_data_e {
PROC_STAT_PEAKSTACK,
PROC_STAT_CURRHEAP,
PROC_STAT_PEAKHEAP,
#ifdef CONFIG_SMP
PROC_STAT_CPU,
#endif
#ifdef CONFIG_SCHED_CPULOAD
#ifdef CONFIG_SCHED_MULTI_CPULOAD
PROC_STAT_CPULOAD_SHORT,
Expand Down
24 changes: 6 additions & 18 deletions apps/system/utils/utils_ps.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ static const char *utils_statenames[] = {
"INVALID ",
"PENDING ",
"READY ",
#ifdef CONFIG_SMP
"ASSIGNED",
#endif
"ASSIGNED",
"RUNNING ",
"INACTIVE",
"WAITSEM ",
Expand All @@ -98,30 +96,27 @@ static const char *utils_ttypenames[4] = {
"--?-- "
};

#ifdef CONFIG_SMP
static uint32_t cpu;
#endif

static void ps_print_values(char *buf, void *arg)
{
int i;
int flags;
int state;
stat_data stat_info[PROC_STAT_MAX];
int ncpus = sched_getcpucount();

stat_info[0] = buf;

for (i = 0; i < PROC_STAT_MAX - 1; i++) {
stat_info[i] = strtok_r(stat_info[i], " ", &stat_info[i + 1]);
}

#ifdef CONFIG_SMP
int cpu_idx;
cpu_idx = atoi(stat_info[PROC_STAT_CPU]);
if (cpu_idx != cpu && cpu != CONFIG_SMP_NCPUS) {
if (cpu_idx != cpu && cpu != ncpus) {
return;
}
#endif

flags = atoi(stat_info[PROC_STAT_FLAG]);
if (flags <= 0) {
Expand All @@ -138,11 +133,7 @@ static void ps_print_values(char *buf, void *arg)
flags & TCB_FLAG_NONCANCELABLE ? 'N' : ' ', flags & TCB_FLAG_CANCEL_PENDING ? 'P' : ' ', \
utils_statenames[state]);

#ifdef CONFIG_SMP
printf(" | %3s", stat_info[PROC_STAT_CPU]);
#else
printf(" | NA ");
#endif

#if (CONFIG_TASK_NAME_SIZE > 0)
printf(" | %s\n", stat_info[PROC_STAT_NAME]);
Expand Down Expand Up @@ -171,6 +162,7 @@ static int ps_read_proc(FAR struct dirent *entryp, FAR void *arg)

int utils_ps(int argc, char **args)
{
int ncpus = sched_getcpucount();
#if !defined(CONFIG_FS_AUTOMOUNT_PROCFS)
int ret;
bool is_mounted;
Expand All @@ -190,9 +182,8 @@ int utils_ps(int argc, char **args)

#endif

#ifdef CONFIG_SMP
int opt;
cpu = CONFIG_SMP_NCPUS;
cpu = ncpus;
if (argc > 1) {
/*
* -c [cpu idx] : only display processes running on given cpu
Expand All @@ -204,7 +195,7 @@ int utils_ps(int argc, char **args)
switch (opt) {
case 'c':
cpu = atoi(optarg);
if (cpu < 0 || cpu >= CONFIG_SMP_NCPUS) {
if (cpu < 0 || cpu >= ncpus) {
printf("Invalid input for -c option\n");
goto out;
}
Expand All @@ -216,17 +207,14 @@ int utils_ps(int argc, char **args)
}
}
}
#endif

printf("\n");
printf(" PID | PRIO | FLAG | TYPE | NP | STATUS | CPU | NAME\n");
printf("------|------|------|---------|----|----------|-----|----\n");
/* Print information for each task/thread */
utils_proc_pid_foreach(ps_read_proc, NULL);

#ifdef CONFIG_SMP
out:
#endif
#if !defined(CONFIG_FS_AUTOMOUNT_PROCFS)
if (!is_mounted) {
/* Detach mounted Procfs */
Expand Down
2 changes: 1 addition & 1 deletion os/fs/procfs/fs_procfsproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ static ssize_t proc_entry_stat(FAR struct proc_file_s *procfile, FAR struct tcb_
#ifdef CONFIG_SMP
linesize = snprintf(procfile->line, STATUS_LINELEN, "%d %d %d %d %d %d %d %d %d %d", tcb->pid, ppid, tcb->sched_priority, tcb->flags, tcb->task_state, tcb->adj_stack_size, peak_stack, curr_heap, peak_heap, tcb->cpu);
#else
linesize = snprintf(procfile->line, STATUS_LINELEN, "%d %d %d %d %d %d %d %d %d", tcb->pid, ppid, tcb->sched_priority, tcb->flags, tcb->task_state, tcb->adj_stack_size, peak_stack, curr_heap, peak_heap);
linesize = snprintf(procfile->line, STATUS_LINELEN, "%d %d %d %d %d %d %d %d %d %d", tcb->pid, ppid, tcb->sched_priority, tcb->flags, tcb->task_state, tcb->adj_stack_size, peak_stack, curr_heap, peak_heap, 0);
#endif
copysize = procfs_memcpy(procfile->line, linesize, buffer, buflen, &offset);
totalsize += copysize;
Expand Down
1 change: 1 addition & 0 deletions os/include/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ int sched_getaffinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask);
int sched_cpucount(FAR const cpu_set_t *set);
int sched_getcpu(void);
#endif /* CONFIG_SMP */
int sched_getcpucount(void);

/* Task Switching Interfaces (non-standard) */
/**
Expand Down
3 changes: 2 additions & 1 deletion os/include/sys/syscall.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@
#define SYS_sched_getaffinity (CONFIG_SYS_RESERVED + 14)
#define SYS_sched_setaffinity (CONFIG_SYS_RESERVED + 15)
#define SYS_sched_getcpu (CONFIG_SYS_RESERVED + 16)
#define __SYS_sem (CONFIG_SYS_RESERVED + 17)
#define SYS_sched_getcpucount (CONFIG_SYS_RESERVED + 17)
#define __SYS_sem (CONFIG_SYS_RESERVED + 18)

/* Semaphores */

Expand Down
2 changes: 0 additions & 2 deletions os/include/tinyara/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,7 @@ enum tstate_e {
TSTATE_TASK_INVALID = 0, /* INVALID - The TCB is uninitialized */
TSTATE_TASK_PENDING, /* READY_TO_RUN - Pending preemption unlock */
TSTATE_TASK_READYTORUN, /* READY-TO-RUN - But not running */
#ifdef CONFIG_SMP
TSTATE_TASK_ASSIGNED, /* READY-TO-RUN - Not running, but assigned to a CPU */
#endif
TSTATE_TASK_RUNNING, /* READY_TO_RUN - And running */

TSTATE_TASK_INACTIVE, /* BLOCKED - Initialized but not yet activated */
Expand Down
4 changes: 4 additions & 0 deletions os/kernel/init/os_start.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,10 @@ const struct tasklist_s g_tasklisttable[NUM_TASK_STATES] =
&g_readytorun,
TLIST_ATTR_PRIORITIZED | TLIST_ATTR_RUNNABLE
},
{ /* TSTATE_TASK_ASSIGNED */
&g_readytorun,
TLIST_ATTR_PRIORITIZED | TLIST_ATTR_RUNNABLE
},
{ /* TSTATE_TASK_RUNNING */
&g_readytorun,
TLIST_ATTR_PRIORITIZED | TLIST_ATTR_RUNNABLE
Expand Down
5 changes: 5 additions & 0 deletions os/kernel/sched/sched_getcpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,8 @@ int sched_getcpu(void)
{
return up_cpu_index(); /* Does not fail */
}

int sched_getcpucount(void)
{
return CONFIG_SMP_NCPUS;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");

}
1 change: 1 addition & 0 deletions os/syscall/syscall.csv
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
"sched_getaffinity","sched.h","","int","pid_t","size_t","FAR cpu_set_t *"
"sched_setaffinity","sched.h","","int","pid_t","size_t","FAR const cpu_set_t*"
"sched_getcpu", "sched.h", "", "int"
"sched_getcpucount" , "sched.h", "", "int"
"sched_getparam", "sched.h", "", "int", "pid_t", "struct sched_param*"
"sched_getscheduler", "sched.h", "", "int", "pid_t"
"sched_getstreams", "tinyara/sched.h", "CONFIG_NFILE_DESCRIPTORS > 0 && CONFIG_NFILE_STREAMS > 0", "FAR struct streamlist*"
Expand Down
1 change: 1 addition & 0 deletions os/syscall/syscall_lookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ SYSCALL_LOOKUP(set_errno, 1, STUB_set_errno)
SYSCALL_LOOKUP(sched_getaffinity, 3, STUB_sched_getaffinity)
SYSCALL_LOOKUP(sched_setaffinity, 3, STUB_sched_setaffinity)
SYSCALL_LOOKUP(sched_getcpu, 0, STUB_sched_getcpu)
SYSCALL_LOOKUP(sched_getcpucount, 0, STUB_sched_getcpucount)

/* Semaphores */

Expand Down
2 changes: 1 addition & 1 deletion os/syscall/syscall_stublookup.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ uintptr_t STUB_sched_setscheduler(int nbr, uintptr_t parm1, uintptr_t parm2,
int STUB_sched_getaffinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask);
int STUB_sched_setaffinity(pid_t pid, size_t cpusetsize, FAR const cpu_set_t *mask);
int STUB_sched_getcpu(void);
int STUB_sched_getcpucount(void);

uintptr_t STUB_sched_unlock(int nbr);
uintptr_t STUB_sched_yield(int nbr);

/* Semaphores */

uintptr_t STUB_sem_close(int nbr, uintptr_t parm1);
Expand Down