Yes, we need revise all enter/leave_critical_section, and do the different thing case by case: 1.Remove the unnecessary enter/leave_critical_section like syslog case 2.Replace with the more fine-grained lock e.g.: a.pthread_mutex/pthread_spin_lock/pthread_rw_mutex for user space b.semaphore/spin_lock for kernel space c.standard atomic operation(yes, we may need port to the old toolchain) Since more and more project start to use the multiple core SoC(at least Sony and Xiaomi), we need fix this problem in a system way: not only address the compile/link issue, but also the performance issue. BTW, sched_lock/sched_unlock also need to revise again. A simple search show up: enter/leave_critical_section 2132 caller in 858 files sched_lock/unlock 251 caller in 173 files Yes, it isn't a simple work, but we need to make a change here if we want to make NuttX become the best SMP RTOS.
Thanks Xiang -----Original Message----- From: spudaneco <spudan...@gmail.com> Sent: Tuesday, May 12, 2020 12:52 PM To: dev@nuttx.apache.org Cc: Nakamura, Yuuichi (Sony) <yuuichi.a.nakam...@sony.com> Subject: RE: enter/leave_critical_section() calls in the user space library I don't understand why you need to do that.But for the SMP case there is a better, lighter weight critical section called spinlock_irqsave. It is intended to protect very short code sequences with a spin lock. Very efficient.enter_critical_section is a big, fat pig in SMP and the lighter spinlock should be used instead.The only thing is that the protected code segment must not suspend itself while holding the spinlock.GregSent from Samsung tablet. -------- Original message --------From: "Nakamura, Yuuichi (Sony)" <yuuichi.a.nakam...@sony.com> Date: 5/11/20 10:34 PM (GMT-06:00) To: dev@nuttx.apache.org Cc: "Nakamura, Yuuichi (Sony)" <yuuichi.a.nakam...@sony.com> Subject: RE: enter/leave_critical_section() calls in the user space library Thanks for accepting my PR.Now it's ok for protected build with uniprocessor case, but not sufficient in SMP case.The following files included in the user mode library contain enter/leave_critical_section() only when CONFIG_SMP=y.libs/libc/misc/lib_filesem.cmm/mm_heap/mm_sem.cThese may have to be moved into the kernel space.Thanks,Yuuichi Nakamura> -----Original Message-----> From: Xiang Xiao <xiaoxiang781...@gmail.com>> Sent: Monday, May 11, 2020 3:20 PM> To: dev@nuttx.apache.org> Subject: RE: enter/leave_critical_section() calls in the user space library> > But we need to consider syslog call from interrupt context too. So I think the> simple modification is:> 1.Drop the critical section> 2.Keep the global variable> In FLAT/PROTECTED build, all user space applications have to share the same> variable.> > -----Original Message-----> From: Gregory Nutt <spudan...@gmail.com>> Sent: Monday, May 11, 2020 12:35 PM> To: dev@nuttx.apache.org> Subject: Re: enter/leave_critical_section() calls in the user space library> > > > From this link:> > https://linux.die.net/man/3/setlogmask> > logmask has per task scope not per thread scope. So we still suffer the> concurrent issue if we change to group-specific logmask.> > The same text appears in the more authoritative Opengroup.org web page:> https://pubs.opengroup.org/onlinepubs/7908799/xsh/closelog.html> > We meet that requirement in the KERNEL mode, but not in the FLAT or> PROTECTED modes.> > So the logmask must be in the group structure and setlogmask must become a> system call at least in the FLAT and PROTECTED build. In the KERNEL build,> a plain global variable as it is now is just fine. There will be one instance of the> global variable in each process address space.> > pthread APIs are not appropriate for use across processes (groups), so we> must avoid pthread mutexes. This will not work in the KERNEL build mode> because each mutex lies in a different address space. A usable mutex would> have to use something like sem_open() which creates a semaphore can be> used across process boundaries.> > Note that setlogmask is classified as NOT thread safe: Preliminary: |> MT-Unsafe race:LogMask | AS-Unsafe | AC-Safe | See POSIX Safety Concepts> <https://www.gnu.org/software/libc/manual/html_node/POSIX-Safety-Con> cepts.html#POSIX-Safety-Concepts>.> See https://www.gnu.org/software/libc/manual/html_node/setlogmask.html> > So we should have no concern about thread safety. No re-entrancy protection> is required. We can just remove the critical section. We do not need to use a> semaphore or mutex.> > Greg>
