Greg, I have to get something out the door today, but I'll be back on this tomorrow and will contribute to your branch.
I don't mind implementing it however the right way is - I just don't necessarily know the right way. So I'll follow your lead. I also need to get caught up to master anyway. My project is still on the old Bitbucket stuff cause we just haven't had time to do the maintenance. Best, Anthony [image: photo] *Anthony Merlino* CTO & Co-founder, Verge Aero (609)-319-1399 On Mon, Jun 29, 2020 at 11:08 AM Gregory Nutt <spudan...@gmail.com> wrote: > > >> > >>> Would userpace_s need to have an additional function pointer for > >>> pthread_cleanup? Similar to pthread_startup? > >>> > >>> Is the up_pthread_start/pthread_startup functions a good example to > >>> follow > >>> for how to call in user mode? > >> > >> If you look at Issue 1263, that is not what I am proposing: > >> https://github.com/apache/incubator-nuttx/issues/1263. That is > >> similar to the solution in the TODO list. But it is overly complex > >> and would not work for KERNEL mode where there is no userspace_s. > >> > >> I am rather proposing dividing pthread_exit() into two pieces: A user > >> space function called pthread_exit() the resides in > >> libs/lib/pthread/lib_pthread_exit.c and a kernel space functions, say > >> nxthread_exit() that resides in sched/pthread/pthread_exit.c: > >> > >> 1. pthread_exit() in libs/lib/pthread/lib_pthread_exit.c would > >> execute when the thread exits. It would only do the things that need > >> to be done in the same context as the pthread execution environment. > >> This would including executing all of the pthread_cleanup functions. > >> That would would have to be migrated from > >> sched/pthread/pthread_exit.c. It would also include the new logic to > >> execute the the new pthread-specific data destructors. And, finally, > >> is would all the nxthread_exit() system function. > >> > >> 2. nxthread_exit() would become a system call (replacing the existing > >> system pthread_exit() system call in syscall/ and in > >> include/sys/syscall*.h. > >> > >> 3. nxthread_exit() would essentially be the existing pthread exit > >> logic sans pthread_cleanup function calls. > >> > >> This sounds straight forward, but there is a complication: We also > >> have to catch the case were the pthread main function does not call > >> pthread_exit() but instead just returns. Currently that is handled > >> in the function pthread_start() in sched/pthread/pthread_create.c. > >> In FLAT mode, it just calls the pthread main function; in other modes > >> it calls up_pthread_start() which will switch to user mode. > >> > >> I have not thought through all of the details, but this would require > >> changes like: > >> > >> 1. Instead of calling the pthread main function, pthread_start() > >> would call a trampoline function in user-space. This already happens > >> in the PROTECTED mode via the pthread_startup() function in the > >> userspace_s structure. But not in KERNEL mode. In KERNEL mode, it > >> uses the SYS_pthread_start system call to start the pthread main > >> function directly. > >> > >> 2. pthread_start would not call pthread_exit(). > >> > >> 3. The trampoline function in user space would simply call the > >> pthread main function and call pthread_exit() is the pthread main > >> function returns. > >> > >> Hmm.. The kernel gets the address of the pthread trampoline from the > >> userspace_s structure. How would the kernel know the address of a > >> process specific pthread thread trampoline function in the KERNEL > >> mode? I am not sure righit now. > >> > >> The design that you describe is probably easier to define. But one > >> objective that I have is to move all of the pthread logic out of the > >> kernel and into libs/libs/pthread. The pthreads interfaces are not > >> part of the kernel in other Unix-like implementations. The kernel > >> does provide certain necessary hooks, but the majority of the > >> implementation of the pthread logic is implemented in libpthread, in > >> user-space and not within the kernel. This is a model that I think > >> that NuttX should follow as well, but things are not well structured > >> to do that now. > > > > "Hmm.. The kernel gets the address of the pthread trampoline from the > > userspace_s structure. How would the kernel know the address of a > > process specific pthread thread trampoline function in the KERNEL > > mode? I am not sure righit now." ... Yes, after sleeping on it, I > > know what would need to happen: > > > > 1. Rename pthread_create() in sched/pthread/pthread_create.c to > > nxthread_create(). Add on new parameter: The address of the > > user-space pthread startup function. Instead of calling the pthread > > main entry (directly or indirectly), pthread_start() would scall the > > pthread startup function, passing it the real address of the pthread > > main function. > > > > The call to pthread_exist would be removed from pthread_startup() and > > move into a new function in user space. > > > > 2. Add libs/libc/pthread/lib_pthread_start.c that would contain two > > trivial functions: > > > > static void pthread_startup(pthread_startroutine_t startroutine, > > pthread_addr_t arg) > > { > > pthread_exit(startroutine(arg)); > > } > > > > int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr, > > pthread_startroutine_t startroutine, pthread_addr_t > > arg) > > { > > return nxthread_create(pthread_startup, thread, attr, startroutine, > > arg); > > } > > > > And that should handle all cases. > > > I created draft PR 1328 that adds the basic changes to move the > trampoline into user space. Please help be contributing to this > branch. You are a committer so you should have privileges to modify my > nuttx_incubator/pthread branch. > > >
