From 5b385f4d4d033d8bee98b2f3c7274363ef8be159 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 16 Oct 2017 11:38:00 -0600 Subject: [PATCH] kthread_create(): Rename kernel_thread() to kthread_create() for better naming consistency with task_create() and kthread_delete(). --- TODO | 36 ++++++++++++++++++------ configs/photon/src/stm32_wdt.c | 8 +++--- configs/sam4s-xplained-pro/src/sam_wdt.c | 8 +++--- configs/stm3240g-eval/src/stm32_boot.c | 6 ++-- drivers/usbdev/usbmsc.c | 8 +++--- drivers/usbhost/usbhost_hidkbd.c | 9 +++--- drivers/usbhost/usbhost_hidmouse.c | 9 +++--- drivers/usbhost/usbhost_xboxcontroller.c | 9 +++--- drivers/usbmonitor/usbmonitor.c | 8 +++--- drivers/wireless/ieee80211/bcmf_sdio.c | 6 ++-- graphics/nxmu/nx_start.c | 4 +-- graphics/vnc/server/vnc_fbdev.c | 2 +- include/nuttx/kthread.h | 6 ++-- sched/init/os_bringup.c | 8 +++--- sched/task/task_create.c | 8 +++--- sched/task/task_posixspawn.c | 8 +++--- sched/wqueue/kwork_hpthread.c | 10 +++---- sched/wqueue/kwork_lpthread.c | 10 +++---- 18 files changed, 93 insertions(+), 70 deletions(-) diff --git a/TODO b/TODO index 8725d5ec3d..74336aef69 100644 --- a/TODO +++ b/TODO @@ -716,9 +716,10 @@ o Kernel/Protected Build Obviously system performance could be improved greatly by simply optimizing these functions so that they do not need to system calls - so frequently. getpid() is (I believe) part of the re-entrant - semaphore logic. Something like TLS might be used to retain the - thread's ID locally. + so frequently. This getpid() call is part of the re-entrant + semaphore logic used with printf() and other C buffered I/O. + Something like TLS might be used to retain the thread's ID + locally. Linux, for example, has functions call up() and down(). up() increments the semaphore count but does not call into the kernel @@ -727,11 +728,11 @@ o Kernel/Protected Build the count becomes negative the caller must be blocked. Update: - "I am thinking that there should be a "magic" global, user-accessible - variable that holds the PID of the currently executing thread; - basically the PID of the task at the head of the ready-to-run list. - This variable would have to be reset each time the head of the ready- - to-run list changes. + "I am thinking that there should be a "magic" global, user- + accessible variable that holds the PID of the currently + executing thread; basically the PID of the task at the head + of the ready-to-run list. This variable would have to be reset + each time the head of the ready-to-run list changes. "Then getpid() could be implemented in user space with no system call by simply reading this variable. @@ -749,6 +750,21 @@ o Kernel/Protected Build Update: One solution might be to used CONFIG_TLS, add the PID to struct tls_info_s. Then the PID could be obtained without a system call. + TLS is not very useful in the FLAT build, however. TLS works by + putting per-thread data at the bottom of an aligned stack. The + current stack pointer is the ANDed with the alignment mask to + obtain the per-thread data address. + + There are problems with this in the FLAT and PROTECTED builds: + First the maximum size of the stack is limited by the number + of bits in the mask. This means that you need to have a very + high alignment to support tasks with large stacks. But + secondly, the higher the alignment of the stacks stacks, the + more memory is lost to fragmentation. + + In the KERNEL build, the the stack lies at a virtual address + and it is possible to have highly aligned stacks with no such + penalties. Status: Open Priority: Low-Medium. Right now, I do not know if these syscalls are a real performance issue or not. The above statistics were collected @@ -798,6 +814,10 @@ o Kernel/Protected Build might also want to be able to modify privileged threads from user tasks with certain permissions. Permissions is a much more complex issue. + + task_delete(), for example, is not permitted to kill a kernel + thread. But should not a privileged user task be able to do + so? Status: Open Priority: Low for most embedded systems but would be a critical need if NuttX were used in a secure system. diff --git a/configs/photon/src/stm32_wdt.c b/configs/photon/src/stm32_wdt.c index 3f2c4fd4f5..f8c0b36034 100644 --- a/configs/photon/src/stm32_wdt.c +++ b/configs/photon/src/stm32_wdt.c @@ -158,10 +158,10 @@ int photon_watchdog_initialize(void) /* Spawn wdog deamon thread */ - int taskid = kernel_thread(CONFIG_PHOTON_WDG_THREAD_NAME, - CONFIG_PHOTON_WDG_THREAD_PRIORITY, - CONFIG_PHOTON_WDG_THREAD_STACKSIZE, - (main_t)wdog_daemon, (FAR char * const *)NULL); + int taskid = kthread_create(CONFIG_PHOTON_WDG_THREAD_NAME, + CONFIG_PHOTON_WDG_THREAD_PRIORITY, + CONFIG_PHOTON_WDG_THREAD_STACKSIZE, + (main_t)wdog_daemon, (FAR char * const *)NULL); if (taskid <= 0) { diff --git a/configs/sam4s-xplained-pro/src/sam_wdt.c b/configs/sam4s-xplained-pro/src/sam_wdt.c index 180d69de90..f405e9b761 100644 --- a/configs/sam4s-xplained-pro/src/sam_wdt.c +++ b/configs/sam4s-xplained-pro/src/sam_wdt.c @@ -192,10 +192,10 @@ int sam_watchdog_initialize(void) #if defined(CONFIG_WDT_THREAD) sched_lock(); - int taskid = kernel_thread(CONFIG_WDT_THREAD_NAME, - CONFIG_WDT_THREAD_PRIORITY, - CONFIG_WDT_THREAD_STACKSIZE, - (main_t)wdog_daemon, (FAR char * const *)NULL); + int taskid = kthread_create(CONFIG_WDT_THREAD_NAME, + CONFIG_WDT_THREAD_PRIORITY, + CONFIG_WDT_THREAD_STACKSIZE, + (main_t)wdog_daemon, (FAR char * const *)NULL); ASSERT(taskid > 0); sched_unlock(); diff --git a/configs/stm3240g-eval/src/stm32_boot.c b/configs/stm3240g-eval/src/stm32_boot.c index 96b5e39bb6..13f7591a40 100644 --- a/configs/stm3240g-eval/src/stm32_boot.c +++ b/configs/stm3240g-eval/src/stm32_boot.c @@ -252,9 +252,9 @@ void board_initialize(void) /* Start the board initialization kernel thread */ - server = kernel_thread("Board Init", CONFIG_STM3240G_BOARDINIT_PRIO, - CONFIG_STM3240G_BOARDINIT_STACK, board_initthread, - NULL); + server = kthread_create("Board Init", CONFIG_STM3240G_BOARDINIT_PRIO, + CONFIG_STM3240G_BOARDINIT_STACK, board_initthread, + NULL); ASSERT(server > 0); #else (void)stm32_bringup(); diff --git a/drivers/usbdev/usbmsc.c b/drivers/usbdev/usbmsc.c index 2f645f0acb..7fdfcbfc88 100644 --- a/drivers/usbdev/usbmsc.c +++ b/drivers/usbdev/usbmsc.c @@ -1704,9 +1704,9 @@ int usbmsc_exportluns(FAR void *handle) g_usbmsc_handoff = priv; uinfo("Starting SCSI worker thread\n"); - priv->thpid = kernel_thread("scsid", CONFIG_USBMSC_SCSI_PRIO, - CONFIG_USBMSC_SCSI_STACKSIZE, - usbmsc_scsi_main, NULL); + priv->thpid = kthread_create("scsid", CONFIG_USBMSC_SCSI_PRIO, + CONFIG_USBMSC_SCSI_STACKSIZE, + usbmsc_scsi_main, NULL); if (priv->thpid <= 0) { usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_THREADCREATE), @@ -1960,4 +1960,4 @@ void usbmsc_get_composite_devdesc(FAR struct composite_devdesc_s *dev) dev->devinfo.nendpoints = USBMSC_NENDPOINTS; } -#endif \ No newline at end of file +#endif diff --git a/drivers/usbhost/usbhost_hidkbd.c b/drivers/usbhost/usbhost_hidkbd.c index 2148d47a0a..39cbdadfb3 100644 --- a/drivers/usbhost/usbhost_hidkbd.c +++ b/drivers/usbhost/usbhost_hidkbd.c @@ -1613,7 +1613,7 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) uinfo("Start poll task\n"); - /* The inputs to a task started by kernel_thread() are very awkard for this + /* The inputs to a task started by kthread_create() are very awkard for this * purpose. They are really designed for command line tasks (argc/argv). So * the following is kludge pass binary data when the keyboard poll task * is started. @@ -1625,9 +1625,10 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) usbhost_takesem(&g_exclsem); g_priv = priv; - priv->pollpid = kernel_thread("kbdpoll", CONFIG_HIDKBD_DEFPRIO, - CONFIG_HIDKBD_STACKSIZE, - (main_t)usbhost_kbdpoll, (FAR char * const *)NULL); + priv->pollpid = kthread_create("kbdpoll", CONFIG_HIDKBD_DEFPRIO, + CONFIG_HIDKBD_STACKSIZE, + (main_t)usbhost_kbdpoll, + (FAR char * const *)NULL); if (priv->pollpid == ERROR) { /* Failed to started the poll thread... probably due to memory resources */ diff --git a/drivers/usbhost/usbhost_hidmouse.c b/drivers/usbhost/usbhost_hidmouse.c index 32c00c302f..6e6b545710 100644 --- a/drivers/usbhost/usbhost_hidmouse.c +++ b/drivers/usbhost/usbhost_hidmouse.c @@ -1683,7 +1683,7 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) uinfo("Start poll task\n"); - /* The inputs to a task started by kernel_thread() are very awkward for this + /* The inputs to a task started by kthread_create() are very awkward for this * purpose. They are really designed for command line tasks (argc/argv). So * the following is kludge pass binary data when the mouse poll task * is started. @@ -1695,9 +1695,10 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) usbhost_takesem(&g_exclsem); g_priv = priv; - priv->pollpid = kernel_thread("mouse", CONFIG_HIDMOUSE_DEFPRIO, - CONFIG_HIDMOUSE_STACKSIZE, - (main_t)usbhost_mouse_poll, (FAR char * const *)NULL); + priv->pollpid = kthread_create("mouse", CONFIG_HIDMOUSE_DEFPRIO, + CONFIG_HIDMOUSE_STACKSIZE, + (main_t)usbhost_mouse_poll, + (FAR char * const *)NULL); if (priv->pollpid == ERROR) { /* Failed to started the poll thread... probably due to memory resources */ diff --git a/drivers/usbhost/usbhost_xboxcontroller.c b/drivers/usbhost/usbhost_xboxcontroller.c index ec8a2c48df..d64ab3353f 100644 --- a/drivers/usbhost/usbhost_xboxcontroller.c +++ b/drivers/usbhost/usbhost_xboxcontroller.c @@ -1324,7 +1324,7 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) * memory resources, primarily for the dedicated stack (CONFIG_XBOXCONTROLLER_STACKSIZE). */ - /* The inputs to a task started by kernel_thread() are very awkward for this + /* The inputs to a task started by kthread_create() are very awkward for this * purpose. They are really designed for command line tasks (argc/argv). So * the following is kludge pass binary data when the controller poll task * is started. @@ -1337,9 +1337,10 @@ static inline int usbhost_devinit(FAR struct usbhost_state_s *priv) g_priv = priv; uinfo("Starting thread\n"); - priv->pollpid = kernel_thread("xbox", CONFIG_XBOXCONTROLLER_DEFPRIO, - CONFIG_XBOXCONTROLLER_STACKSIZE, - (main_t)usbhost_xboxcontroller_poll, (FAR char * const *)NULL); + priv->pollpid = kthread_create("xbox", CONFIG_XBOXCONTROLLER_DEFPRIO, + CONFIG_XBOXCONTROLLER_STACKSIZE, + (main_t)usbhost_xboxcontroller_poll, + (FAR char * const *)NULL); if (priv->pollpid == ERROR) { /* Failed to started the poll thread... probably due to memory resources */ diff --git a/drivers/usbmonitor/usbmonitor.c b/drivers/usbmonitor/usbmonitor.c index 1706837280..6d350f23db 100644 --- a/drivers/usbmonitor/usbmonitor.c +++ b/drivers/usbmonitor/usbmonitor.c @@ -225,10 +225,10 @@ int usbmonitor_start(void) g_usbmonitor.started = true; g_usbmonitor.stop = false; - ret = kernel_thread("USB Monitor", CONFIG_USBMONITOR_PRIORITY, - CONFIG_USBMONITOR_STACKSIZE, - (main_t)usbmonitor_daemon, - (FAR char * const *)NULL); + ret = kthread_create("USB Monitor", CONFIG_USBMONITOR_PRIORITY, + CONFIG_USBMONITOR_STACKSIZE, + (main_t)usbmonitor_daemon, + (FAR char * const *)NULL); if (ret < 0) { int errcode = errno; diff --git a/drivers/wireless/ieee80211/bcmf_sdio.c b/drivers/wireless/ieee80211/bcmf_sdio.c index 2742b85bfe..25f94ab303 100644 --- a/drivers/wireless/ieee80211/bcmf_sdio.c +++ b/drivers/wireless/ieee80211/bcmf_sdio.c @@ -670,9 +670,9 @@ int bcmf_bus_sdio_initialize(FAR struct bcmf_dev_s *priv, /* Spawn bcmf daemon thread */ - ret = kernel_thread(BCMF_THREAD_NAME, SCHED_PRIORITY_MAX, - BCMF_THREAD_STACK_SIZE, bcmf_sdio_thread, - (FAR char * const *)NULL); + ret = kthread_create(BCMF_THREAD_NAME, SCHED_PRIORITY_MAX, + BCMF_THREAD_STACK_SIZE, bcmf_sdio_thread, + (FAR char * const *)NULL); if (ret <= 0) { diff --git a/graphics/nxmu/nx_start.c b/graphics/nxmu/nx_start.c index 1d2565bb01..e2f53952e4 100644 --- a/graphics/nxmu/nx_start.c +++ b/graphics/nxmu/nx_start.c @@ -191,8 +191,8 @@ int nx_start(void) /* Start the server kernel thread */ ginfo("Starting server task\n"); - server = kernel_thread("NX Server", CONFIG_NXSTART_SERVERPRIO, - CONFIG_NXSTART_SERVERSTACK, nx_server, NULL); + server = kthread_create("NX Server", CONFIG_NXSTART_SERVERPRIO, + CONFIG_NXSTART_SERVERSTACK, nx_server, NULL); if (server < 0) { int errcode = errno; diff --git a/graphics/vnc/server/vnc_fbdev.c b/graphics/vnc/server/vnc_fbdev.c index 0683619b6c..e599599654 100644 --- a/graphics/vnc/server/vnc_fbdev.c +++ b/graphics/vnc/server/vnc_fbdev.c @@ -452,7 +452,7 @@ static int vnc_start_server(int display) argv[0] = str; argv[1] = NULL; - pid = kernel_thread("vnc_server", CONFIG_VNCSERVER_PRIO, + pid = kthread_create("vnc_server", CONFIG_VNCSERVER_PRIO, CONFIG_VNCSERVER_STACKSIZE, (main_t)vnc_server, argv); if (pid < 0) diff --git a/include/nuttx/kthread.h b/include/nuttx/kthread.h index 9ce84eeeb1..f345a0e914 100644 --- a/include/nuttx/kthread.h +++ b/include/nuttx/kthread.h @@ -65,7 +65,7 @@ extern "C" ****************************************************************************/ /******************************************************************************** - * Name: kernel_thread + * Name: kthread_create * * Description: * This function creates and activates a kernel thread task with kernel-mode @@ -80,8 +80,8 @@ extern "C" * ********************************************************************************/ -int kernel_thread(FAR const char *name, int priority, int stack_size, - main_t entry, FAR char * const argv[]); +int kthread_create(FAR const char *name, int priority, int stack_size, + main_t entry, FAR char * const argv[]); /**************************************************************************** * Name: kthread_delete diff --git a/sched/init/os_bringup.c b/sched/init/os_bringup.c index 7887ab8770..f2028fc45f 100644 --- a/sched/init/os_bringup.c +++ b/sched/init/os_bringup.c @@ -154,9 +154,9 @@ static inline void os_pgworker(void) sinfo("Starting paging thread\n"); - g_pgworker = kernel_thread("pgfill", CONFIG_PAGING_DEFPRIO, - CONFIG_PAGING_STACKSIZE, - (main_t)pg_worker, (FAR char * const *)NULL); + g_pgworker = kthread_create("pgfill", CONFIG_PAGING_DEFPRIO, + CONFIG_PAGING_STACKSIZE, + (main_t)pg_worker, (FAR char * const *)NULL); DEBUGASSERT(g_pgworker > 0); } @@ -354,7 +354,7 @@ static inline void os_start_application(void) * execution. */ - pid = kernel_thread("AppBringUp", CONFIG_BOARD_INITTHREAD_PRIORITY, + pid = kthread_create("AppBringUp", CONFIG_BOARD_INITTHREAD_PRIORITY, CONFIG_BOARD_INITTHREAD_STACKSIZE, (main_t)os_start_task, (FAR char * const *)NULL); ASSERT(pid > 0); diff --git a/sched/task/task_create.c b/sched/task/task_create.c index f87d2f3f53..a1d07e1d8e 100644 --- a/sched/task/task_create.c +++ b/sched/task/task_create.c @@ -62,7 +62,7 @@ * Description: * This function creates and activates a new thread of the specified type * with a specified priority and returns its system-assigned ID. It is the - * internal, common implementation of task_create() and kernel_thread(). + * internal, common implementation of task_create() and kthread_create(). * See comments with task_create() for further information. * * Input Parameters: @@ -243,7 +243,7 @@ int task_create(FAR const char *name, int priority, #endif /**************************************************************************** - * Name: kernel_thread + * Name: kthread_create * * Description: * This function creates and activates a kernel thread task with kernel- @@ -258,8 +258,8 @@ int task_create(FAR const char *name, int priority, * ****************************************************************************/ -int kernel_thread(FAR const char *name, int priority, - int stack_size, main_t entry, FAR char *const argv[]) +int kthread_create(FAR const char *name, int priority, + int stack_size, main_t entry, FAR char *const argv[]) { return thread_create(name, TCB_FLAG_TTYPE_KERNEL, priority, stack_size, entry, argv); diff --git a/sched/task/task_posixspawn.c b/sched/task/task_posixspawn.c index b1595acad8..1922690aab 100644 --- a/sched/task/task_posixspawn.c +++ b/sched/task/task_posixspawn.c @@ -414,10 +414,10 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path, * task. */ - proxy = kernel_thread("posix_spawn_proxy", param.sched_priority, - CONFIG_POSIX_SPAWN_PROXY_STACKSIZE, - (main_t)posix_spawn_proxy, - (FAR char * const *)NULL); + proxy = kthread_create("posix_spawn_proxy", param.sched_priority, + CONFIG_POSIX_SPAWN_PROXY_STACKSIZE, + (main_t)posix_spawn_proxy, + (FAR char * const *)NULL); if (proxy < 0) { ret = get_errno(); diff --git a/sched/wqueue/kwork_hpthread.c b/sched/wqueue/kwork_hpthread.c index 4ef84731ea..8a3d3f0e88 100644 --- a/sched/wqueue/kwork_hpthread.c +++ b/sched/wqueue/kwork_hpthread.c @@ -154,10 +154,10 @@ int work_hpstart(void) sinfo("Starting high-priority kernel worker thread\n"); - pid = kernel_thread(HPWORKNAME, CONFIG_SCHED_HPWORKPRIORITY, - CONFIG_SCHED_HPWORKSTACKSIZE, - (main_t)work_hpthread, - (FAR char * const *)NULL); + pid = kthread_create(HPWORKNAME, CONFIG_SCHED_HPWORKPRIORITY, + CONFIG_SCHED_HPWORKSTACKSIZE, + (main_t)work_hpthread, + (FAR char * const *)NULL); DEBUGASSERT(pid > 0); if (pid < 0) @@ -165,7 +165,7 @@ int work_hpstart(void) int errcode = errno; DEBUGASSERT(errcode > 0); - serr("ERROR: kernel_thread failed: %d\n", errcode); + serr("ERROR: kthread_create failed: %d\n", errcode); return -errcode; } diff --git a/sched/wqueue/kwork_lpthread.c b/sched/wqueue/kwork_lpthread.c index a6864379ce..4b6b3ee997 100644 --- a/sched/wqueue/kwork_lpthread.c +++ b/sched/wqueue/kwork_lpthread.c @@ -201,10 +201,10 @@ int work_lpstart(void) for (wndx = 0; wndx < CONFIG_SCHED_LPNTHREADS; wndx++) { - pid = kernel_thread(LPWORKNAME, CONFIG_SCHED_LPWORKPRIORITY, - CONFIG_SCHED_LPWORKSTACKSIZE, - (main_t)work_lpthread, - (FAR char * const *)NULL); + pid = kthread_create(LPWORKNAME, CONFIG_SCHED_LPWORKPRIORITY, + CONFIG_SCHED_LPWORKSTACKSIZE, + (main_t)work_lpthread, + (FAR char * const *)NULL); DEBUGASSERT(pid > 0); if (pid < 0) @@ -212,7 +212,7 @@ int work_lpstart(void) int errcode = errno; DEBUGASSERT(errcode > 0); - serr("ERROR: kernel_thread %d failed: %d\n", wndx, errcode); + serr("ERROR: kthread_create %d failed: %d\n", wndx, errcode); sched_unlock(); return -errcode; }