On 10/25/2023 8:18 AM, Alan C. Assis wrote:
On 10/25/23, Nathan Hartman<hartman.nat...@gmail.com>  wrote:
On Wed, Oct 25, 2023 at 5:16 AM Ville Juven<ville.ju...@gmail.com>  wrote:

Hi all,

I noticed that when spawning a new task/process from a file in
nsh_fileapps, the scheduler is locked prior to calling posix_spawn(),
which
does the file loading into memory etc.

I noticed one issue with this; when the file size is large (in the order
of
MB) the scheduler is locked for very long periods at a time, in the order
of hundreds of milliseconds.


This sounds like a bug. The scheduler should not be locked during IO-bound
operations, since there is no way to know how long they will take. Loading
from flash could take hundreds of milliseconds (which is already terrible)
but imagine a different scenario where loading from a network with
connection problems outside of the device could lock the device for many
seconds!

If I understood this comment correctly:

   /* Lock the scheduler in an attempt to prevent the application from
    * running until waitpid() has been called.
    */

then maybe instead of forcing a sched_lock() we could change the
task_state to TSTATE_TASK_INACTIVE or some other that prevent the task
to be scheduled again before the posix_spawnp() get finished.

BR,

Alan

I think that the sched_lock() is not necessary.   Notice that the this is only "an attempt" to keep the application from running and executing.  Without the sched_lock(), the task may run and exit before there is a change to call waitpid() which should effect only the user experience.

A good test to make sure that still works would be remove the sched_lock/unlock and add a test case like:

   int main(int argc, char **argv)
   {
      return 0;
   }

That is the case that the logic is trying to avoid, but it seems like it should work fine.  Subsequent logic handles this case (but provides no use feedback).  See comments:

          /* Wait for the application to exit.  We did lock the scheduler
           * above, but that does not guarantee that the application did not            * already run to completion in the case where I/O was redirected.
           * Here the scheduler will be unlocked while waitpid is waiting
           * and if the application has not yet run, it will now be able to
           * do so.
           *
           * NOTE: WUNTRACED does nothing in the default case, but in the
           * case the where CONFIG_SIG_SIGSTOP_ACTION=y, the file app
           * may also be stopped.  In that case WUNTRACED will force
           * waitpid() to return with ECHILD.
           */

And

              /* If the child thread doesn't exist, waitpid() will return the
               * error ECHLD.  Since we know that the task was successfully
               * started, this must be one of the cases described above; we
               * have to assume that the task already exit'ed. In this case,
               * we have no idea if the application ran successfully or not
               * (because NuttX does not retain exit status of child tasks).
               * Let's assume that is did run successfully.
               */

So it looks to me like the case where the program exits is already handled.



Reply via email to