sched/task_[posix]spawn: Simplify how spawn attributes are handled

Handle task spawn attributes as task spawn file actions are handled.

Why? This removes the need for sched_lock() when the task is being
spawned. When loading the new task from a file the scheduler can be
locked for a VERY LONG time, in the order of hundreds of milliseconds!

This is unacceptable for real time operation.

Also fixes a latent bug in exec_module, spawn_file_actions is executed
at a bad location; when CONFIG_ARCH_ADDRENV=y actions will point to the
new process's address environment (as it is temporarily instantiated at
that point). Fix this by moving it to after addrenv_restore.

binfmt/binfmt_exec.c

@@ -134,7 +134,7 @@ static int exec_internal(FAR const char *filename,
 
   /* Then start the module */
 
-  pid = exec_module(bin, filename, argv, envp, actions, spawn);
+  pid = exec_module(bin, filename, argv, envp, actions, attr, spawn);
   if (pid < 0)
     {
       ret = pid;

binfmt/binfmt_execmodule.c

@@ -37,6 +37,7 @@
 #include <nuttx/kmalloc.h>
 #include <nuttx/sched.h>
 #include <sched/sched.h>
+#include <task/spawn.h>
 #include <nuttx/spawn.h>
 #include <nuttx/binfmt/binfmt.h>
 
@@ -203,6 +204,7 @@ int exec_module(FAR struct binary_s *binp,
                 FAR const char *filename, FAR char * const *argv,
                 FAR char * const *envp,
                 FAR const posix_spawn_file_actions_t *actions,
+                FAR const posix_spawnattr_t *attr,
                 bool spawn)
 {
   FAR struct task_tcb_s *tcb;
@@ -326,17 +328,6 @@ int exec_module(FAR struct binary_s *binp,
   binfmt_freeargv(argv);
   binfmt_freeenv(envp);
 
-  /* Perform file actions */
-
-  if (actions != NULL)
-    {
-      ret = spawn_file_actions(&tcb->cmn, actions);
-      if (ret < 0)
-        {
-          goto errout_with_tcbinit;
-        }
-    }
-
 #ifdef CONFIG_PIC
   /* Add the D-Space address as the PIC base address.  By convention, this
    * must be the first allocated address space.
@@ -393,10 +384,6 @@ int exec_module(FAR struct binary_s *binp,
 
   pid = tcb->cmn.pid;
 
-  /* Then activate the task at the provided priority */
-
-  nxtask_activate((FAR struct tcb_s *)tcb);
-
 #if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
   /* Restore the address environment of the caller */
 
@@ -408,6 +395,32 @@ int exec_module(FAR struct binary_s *binp,
     }
 #endif
 
+  /* Perform file actions */
+
+  if (actions != NULL)
+    {
+      ret = spawn_file_actions(&tcb->cmn, actions);
+      if (ret < 0)
+        {
+          goto errout_with_tcbinit;
+        }
+    }
+
+  /* Set the attributes */
+
+  if (attr)
+    {
+      ret = spawn_execattrs(pid, attr);
+      if (ret < 0)
+        {
+          goto errout_with_tcbinit;
+        }
+    }
+
+  /* Then activate the task at the provided priority */
+
+  nxtask_activate((FAR struct tcb_s *)tcb);
+
   return pid;
 
 errout_with_tcbinit:

include/nuttx/binfmt/binfmt.h

@@ -271,6 +271,7 @@ int exec_module(FAR struct binary_s *binp,
                 FAR const char *filename, FAR char * const *argv,
                 FAR char * const *envp,
                 FAR const posix_spawn_file_actions_t *actions,
+                FAR const posix_spawnattr_t *attr,
                 bool spawn);
 
 /****************************************************************************

sched/task/task_posixspawn.c

@@ -102,13 +102,6 @@ static int nxposix_spawn_exec(FAR pid_t *pidp, FAR const char *path,
 
   exec_getsymtab(&symtab, &nsymbols);
 
-  /* Disable pre-emption so that we can modify the task parameters after
-   * we start the new task; the new task will not actually begin execution
-   * until we re-enable pre-emption.
-   */
-
-  sched_lock();
-
   /* Start the task */
 
   pid = exec_spawn(path, argv, envp, symtab, nsymbols, actions, attr);
@@ -116,7 +109,7 @@ static int nxposix_spawn_exec(FAR pid_t *pidp, FAR const char *path,
     {
       ret = -pid;
       serr("ERROR: exec failed: %d\n", ret);
-      goto errout;
+      return ret;
     }
 
   /* Return the task ID to the caller */
@@ -126,20 +119,6 @@ static int nxposix_spawn_exec(FAR pid_t *pidp, FAR const char *path,
       *pidp = pid;
     }
 
-  /* Now set the attributes.  Note that we ignore all of the return values
-   * here because we have already successfully started the task.  If we
-   * return an error value, then we would also have to stop the task.
-   */
-
-  if (attr)
-    {
-      spawn_execattrs(pid, attr);
-    }
-
-  /* Re-enable pre-emption and return */
-
-errout:
-  sched_unlock();
   return ret;
 }
 

sched/task/task_spawn.c

@@ -80,7 +80,8 @@ static int nxtask_spawn_create(FAR const char *name, int priority,
                               FAR void *stack_addr, int stack_size,
                               main_t entry, FAR char * const argv[],
                               FAR char * const envp[],
-                              FAR const posix_spawn_file_actions_t *actions)
+                              FAR const posix_spawn_file_actions_t *actions,
+                              FAR const posix_spawnattr_t *attr)
 {
   FAR struct task_tcb_s *tcb;
   pid_t pid;
@@ -109,6 +110,10 @@ static int nxtask_spawn_create(FAR const char *name, int priority,
       return ret;
     }
 
+  /* Get the assigned pid before we start the task */
+
+  pid = tcb->cmn.pid;
+
   /* Perform file actions */
 
   if (actions != NULL)
@@ -116,20 +121,30 @@ static int nxtask_spawn_create(FAR const char *name, int priority,
       ret = spawn_file_actions(&tcb->cmn, actions);
       if (ret < 0)
         {
-          nxtask_uninit(tcb);
+          goto errout_with_taskinit;
-          return ret;
         }
     }
 
-  /* Get the assigned pid before we start the task */
+  /* Set the attributes */
 
-  pid = tcb->cmn.pid;
+  if (attr)
+    {
+      ret = spawn_execattrs(pid, attr);
+      if (ret < 0)
+        {
+          goto errout_with_taskinit;
+        }
+    }
 
   /* Activate the task */
 
   nxtask_activate(&tcb->cmn);
 
   return pid;
+
+errout_with_taskinit:
+  nxtask_uninit(tcb);
+  return ret;
 }
 
 /****************************************************************************
@@ -188,13 +203,6 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
   int pid;
   int ret = OK;
 
-  /* Disable pre-emption so that we can modify the task parameters after
-   * we start the new task; the new task will not actually begin execution
-   * until we re-enable pre-emption.
-   */
-
-  sched_lock();
-
   /* Use the default priority and stack size if no attributes are provided */
 
   if (attr)
@@ -212,7 +220,7 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
       ret = nxsched_get_param(0, &param);
       if (ret < 0)
         {
-          goto errout;
+          return ret;
         }
 
       priority  = param.sched_priority;
@@ -223,12 +231,12 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
 
   pid = nxtask_spawn_create(name, priority, stackaddr,
                             stacksize, entry, argv,
-                            envp ? envp : environ, actions);
+                            envp ? envp : environ, actions, attr);
   if (pid < 0)
     {
       ret = pid;
       serr("ERROR: nxtask_spawn_create failed: %d\n", ret);
-      goto errout;
+      return ret;
     }
 
   /* Return the task ID to the caller */
@@ -238,20 +246,6 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
       *pidp = pid;
     }
 
-  /* Now set the attributes.  Note that we ignore all of the return values
-   * here because we have already successfully started the task.  If we
-   * return an error value, then we would also have to stop the task.
-   */
-
-  if (attr)
-    {
-      spawn_execattrs(pid, attr);
-    }
-
-  /* Re-enable pre-emption and return */
-
-errout:
-  sched_unlock();
   return ret;
 }