Fix bug introduced to msconn; Was hanging is invoked from NSH

This commit is contained in:
Gregory Nutt 2013-09-07 10:09:52 -06:00
parent af051009ba
commit b345e0d8e7
5 changed files with 159 additions and 37 deletions

View File

@ -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).

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -374,10 +374,6 @@ static int usbmsc_enumerate(struct usbtrace_s *trace, void *arg)
}
#endif
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* msconn_main
*
@ -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, &param);
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