diff options
author | Gregory Nutt <gnutt@nuttx.org> | 2013-09-07 10:09:52 -0600 |
---|---|---|
committer | Gregory Nutt <gnutt@nuttx.org> | 2013-09-07 10:09:52 -0600 |
commit | a565e702360db5afd59864a83f6f922b7e8817de (patch) | |
tree | ce85e9eee5125f852a0fd70397bb66e0a2202c08 /apps | |
parent | a166b6f07328bbbfba1ce5811a2d57543cd4b849 (diff) | |
download | nuttx-a565e702360db5afd59864a83f6f922b7e8817de.tar.gz nuttx-a565e702360db5afd59864a83f6f922b7e8817de.tar.bz2 nuttx-a565e702360db5afd59864a83f6f922b7e8817de.zip |
Fix bug introduced to msconn; Was hanging is invoked from NSH
Diffstat (limited to 'apps')
-rw-r--r-- | apps/ChangeLog.txt | 9 | ||||
-rw-r--r-- | apps/examples/usbmsc/Kconfig | 36 | ||||
-rw-r--r-- | apps/examples/usbmsc/Makefile | 11 | ||||
-rw-r--r-- | apps/examples/usbmsc/usbmsc.h | 6 | ||||
-rw-r--r-- | apps/examples/usbmsc/usbmsc_main.c | 134 |
5 files changed, 159 insertions, 37 deletions
diff --git a/apps/ChangeLog.txt b/apps/ChangeLog.txt index 41fa573e4..c91f07574 100644 --- a/apps/ChangeLog.txt +++ b/apps/ChangeLog.txt @@ -632,4 +632,13 @@ an NSH built-in task. In that case our attempts are inadequate and only interfere with with other logic that is attempting to to do the same thing (in NSH or in the USB monitor) (2013-9-6). + * apps/examples/usbmsc: IMPORTANT bug fix: Change how the msconn + works. Because of recent changes the msconn command was hanging. + This was because the USB MSC start-up logic creates a pthread; + Now waitpid() will wait until all members of the task groupd + exit. So NSH was hanging in waitpid when msconn started even + though msconn returned. The USB MSC logic really should not use + a pthread, but we are stuck with that for now. The work-around + is the msconn now daemonizes itself so that it so taht the pthread + is created in a different task group (2013-8-6). diff --git a/apps/examples/usbmsc/Kconfig b/apps/examples/usbmsc/Kconfig index 9940fe482..4d366d89f 100644 --- a/apps/examples/usbmsc/Kconfig +++ b/apps/examples/usbmsc/Kconfig @@ -127,5 +127,41 @@ config EXAMPLES_USBMSC_TRACEINTERRUPTS This setting will show USB device controller interrupt-related events. endif # EXAMPLES_USBMSC_TRACE + +if NSH_BUILTIN_APPS + +config EXAMPLES_USBMSC_CMD_STACKSIZE + int "Stacksize of msconn and msdis commands" + default 768 + ---help--- + Size of the stack used by the small 'msconn' and 'msdis' command + applications. Warning, just because the applications are small, + the stack usage could still be deep! + +config EXAMPLES_USBMSC_CMD_PRIORITY + int "Priority of the msconn and msdis commands" + default 100 + ---help--- + Priority of the small 'msconn' and 'msdis' command applications. + +config EXAMPLES_USBMSC_DAEMON_STACKSIZE + int "Stacksize of msconn daemon" + default 2048 + ---help--- + To avoid threading entanglements, the USB MSC class is initialized + on a daemon thread. This permits the msconn application to return + to the NSH command line immediately. This is the stack used for + that short-lived USB MSC initialization daemon. + +config EXAMPLES_USBMSC_DAEMON_PRIORITY + int "Priority of the msconn daemon" + default 100 + ---help--- + To avoid threading entanglements, the USB MSC class is initialized + on a daemon thread. This permits the msconn application to return + to the NSH command line immediately. This is the priority used for + that short-lived USB MSC initialization daemon. + +endif # NSH_BUILTIN_APPS endif # EXAMPLES_USBMSC diff --git a/apps/examples/usbmsc/Makefile b/apps/examples/usbmsc/Makefile index 2dbf78260..73031e991 100644 --- a/apps/examples/usbmsc/Makefile +++ b/apps/examples/usbmsc/Makefile @@ -62,13 +62,16 @@ ROOTDEPPATH = --dep-path . # USB storage built-in application info +CONFIG_EXAMPLES_USBMSC_CMD_STACKSIZE ?= 768 +CONFIG_EXAMPLES_USBMSC_CMD_PRIORITY ?= SCHED_PRIORITY_DEFAULT + APPNAME1 = msconn -PRIORITY1 = SCHED_PRIORITY_DEFAULT -STACKSIZE1 = 2048 +PRIORITY1 = $(CONFIG_EXAMPLES_USBMSC_CMD_PRIORITY) +STACKSIZE1 = $(CONFIG_EXAMPLES_USBMSC_CMD_STACKSIZE) APPNAME2 = msdis -PRIORITY2 = SCHED_PRIORITY_DEFAULT -STACKSIZE2 = 2048 +PRIORITY2 = $(CONFIG_EXAMPLES_USBMSC_CMD_PRIORITY) +STACKSIZE2 = $(CONFIG_EXAMPLES_USBMSC_CMD_STACKSIZE) # Common build diff --git a/apps/examples/usbmsc/usbmsc.h b/apps/examples/usbmsc/usbmsc.h index 2ae796efc..e0e3ba81b 100644 --- a/apps/examples/usbmsc/usbmsc.h +++ b/apps/examples/usbmsc/usbmsc.h @@ -89,6 +89,12 @@ # undef CONFIG_EXAMPLES_USBMSC_DEVPATH3 #endif +#if defined(CONFIG_NSH_BUILTIN_APPS) && defined(CONFIG_SCHED_WAITPID) +# ifndef CONFIG_EXAMPLES_USBMSC_DAEMON_STACKSIZE +# define CONFIG_EXAMPLES_USBMSC_DAEMON_STACKSIZE 2048 +# endif +#endif + /* Debug ********************************************************************/ #ifdef CONFIG_CPP_HAVE_VARARGS diff --git a/apps/examples/usbmsc/usbmsc_main.c b/apps/examples/usbmsc/usbmsc_main.c index 95875c423..3140182a8 100644 --- a/apps/examples/usbmsc/usbmsc_main.c +++ b/apps/examples/usbmsc/usbmsc_main.c @@ -375,10 +375,6 @@ static int usbmsc_enumerate(struct usbtrace_s *trace, void *arg) #endif /**************************************************************************** - * Public Functions - ****************************************************************************/ - -/**************************************************************************** * msconn_main * * Description: @@ -389,7 +385,7 @@ static int usbmsc_enumerate(struct usbtrace_s *trace, void *arg) * ****************************************************************************/ -int msconn_main(int argc, char *argv[]) +static int msconn_daemon(int argc, char *argv[]) { FAR void *handle; int ret; @@ -399,16 +395,15 @@ int msconn_main(int argc, char *argv[]) */ #ifdef CONFIG_NSH_BUILTIN_APPS + /* Check if there is a non-NULL USB mass storage device handle (meaning that the + * USB mass storage device is already configured). + */ - /* Check if there is a non-NULL USB mass storage device handle (meaning that the - * USB mass storage device is already configured). - */ - - if (g_usbmsc.mshandle) - { - message("msconn_main: ERROR: Already connected\n"); - return 1; - } + if (g_usbmsc.mshandle) + { + message("msconn_main: ERROR: Already connected\n"); + return EXIT_FAILURE; + } #endif #ifdef CONFIG_EXAMPLES_USBMSC_DEBUGMM @@ -435,7 +430,7 @@ int msconn_main(int argc, char *argv[]) if (ret < 0) { message("msconn_main: usbmsc_archinitialize failed: %d\n", -ret); - return 2; + return EXIT_FAILURE; } check_test_memory_usage("After usbmsc_archinitialize()"); @@ -447,7 +442,7 @@ int msconn_main(int argc, char *argv[]) { message("msconn_main: usbmsc_configure failed: %d\n", -ret); usbmsc_uninitialize(handle); - return 3; + return EXIT_FAILURE; } message("msconn_main: handle=%p\n", handle); check_test_memory_usage("After usbmsc_configure()"); @@ -459,7 +454,7 @@ int msconn_main(int argc, char *argv[]) message("msconn_main: usbmsc_bindlun failed for LUN 1 using %s: %d\n", CONFIG_EXAMPLES_USBMSC_DEVPATH1, -ret); usbmsc_uninitialize(handle); - return 4; + return EXIT_FAILURE; } check_test_memory_usage("After usbmsc_bindlun()"); @@ -472,7 +467,7 @@ int msconn_main(int argc, char *argv[]) message("msconn_main: usbmsc_bindlun failed for LUN 2 using %s: %d\n", CONFIG_EXAMPLES_USBMSC_DEVPATH2, -ret); usbmsc_uninitialize(handle); - return 5; + return EXIT_FAILURE; } check_test_memory_usage("After usbmsc_bindlun() #2"); @@ -485,7 +480,7 @@ int msconn_main(int argc, char *argv[]) message("msconn_main: usbmsc_bindlun failed for LUN 3 using %s: %d\n", CONFIG_EXAMPLES_USBMSC_DEVPATH3, -ret); usbmsc_uninitialize(handle); - return 6; + return EXIT_FAILURE; } check_test_memory_usage("After usbmsc_bindlun() #3"); @@ -497,7 +492,7 @@ int msconn_main(int argc, char *argv[]) { message("msconn_main: usbmsc_exportluns failed: %d\n", -ret); usbmsc_uninitialize(handle); - return 7; + return EXIT_FAILURE; } check_test_memory_usage("After usbmsc_exportluns()"); @@ -522,7 +517,7 @@ int msconn_main(int argc, char *argv[]) { message("msconn_main: usbtrace_enumerate failed: %d\n", -ret); usbmsc_uninitialize(handle); - return 8; + return EXIT_FAILURE; } check_test_memory_usage("After usbtrace_enumerate()"); # else @@ -531,25 +526,98 @@ int msconn_main(int argc, char *argv[]) } #elif defined(CONFIG_NSH_BUILTIN_APPS) - /* Return the USB mass storage device handle so it can be used by the 'misconn' - * command. - */ + /* Return the USB mass storage device handle so it can be used by the 'misconn' + * command. + */ - message("msconn_main: Connected\n"); - g_usbmsc.mshandle = handle; - check_test_memory_usage("After MS connection"); + message("msconn_main: Connected\n"); + g_usbmsc.mshandle = handle; + check_test_memory_usage("After MS connection"); #else /* defined(CONFIG_DISABLE_SIGNALS) */ /* Just exit */ - message("msconn_main: Exiting\n"); + message("msconn_main: Exiting\n"); - /* Dump debug memory usage */ + /* Dump debug memory usage */ - final_memory_usage("Final memory usage"); + final_memory_usage("Final memory usage"); +#endif + return EXIT_SUCCESS; +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * msconn_main + * + * Description: + * This is the main program that configures the USB mass storage device + * and exports the LUN(s). If CONFIG_NSH_BUILTIN_APPS is defined + * in the NuttX configuration, then this program can be executed by + * entering the "msconn" command at the NSH console. + * + ****************************************************************************/ + +int msconn_main(int argc, char *argv[]) +{ + /* If this function is started as a built-in application from the NSH + * command line, then daemonize. Why? Because NSH is probably waiting + * on waitpid() for msconn to complete. But the USB MSC initialization + * logic creates a dedicated worker thread using pthread_create(). As + * consequences, there will be two members of the task group and waitpid() + * will not wake up when msconn returns. It will not wake-up until both + * msconn and the USB MSC work thread terminate. + */ + +#if defined(CONFIG_NSH_BUILTIN_APPS) && defined(CONFIG_SCHED_WAITPID) + char *newargv[1] = { NULL }; + struct sched_param param; + int ret; + + /* Check if there is a non-NULL USB mass storage device handle (meaning + * that the USB mass storage device is already configured). There is + * no handshaking so there is a race condition: We will check again + * when the daemon is started. + */ + + if (g_usbmsc.mshandle) + { + message("msconn_main: ERROR: Already connected\n"); + return 1; + } + +#ifndef CONFIG_EXAMPLES_USBMSC_DAEMON_PRIORITY + /* Set the daemon to the same priority as this task */ + + ret = sched_getparam(0, ¶m); + if (ret < 0) + { + message("msconn_main: ERROR: Already connected\n"); + return EXIT_FAILURE; + } +#else + param.sched_priority = CONFIG_EXAMPLES_USBMSC_DAEMON_PRIORITY; +#endif + + ret = TASK_CREATE("msconn daemon", param.sched_priority, + CONFIG_EXAMPLES_USBMSC_DAEMON_STACKSIZE, + msconn_daemon, newargv); + if (ret < 0) + { + message("msconn_main: ERROR: TASK_CREATE failed: %d\n", ret); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +#else + /* Otherwise, there is no need to daemonize */ + + return msconn_daemon(argc, argv); #endif - return 0; } /**************************************************************************** @@ -571,7 +639,7 @@ int msdis_main(int argc, char *argv[]) if (!g_usbmsc.mshandle) { message("msdis: ERROR: Not connected\n"); - return 1; + return EXIT_FAILURE; } check_test_memory_usage("Since MS connection"); @@ -585,6 +653,6 @@ int msdis_main(int argc, char *argv[]) /* Dump debug memory usage */ final_memory_usage("Final memory usage"); - return 0; + return EXIT_SUCCESS; } #endif |