On Wed, Jan 5, 2022 at 10:30 PM Xiang Xiao <xiaoxiang781...@gmail.com> wrote:
> On Thu, Jan 6, 2022 at 4:23 AM Gregory Nutt <spudan...@gmail.com> wrote: > > > > I truly believe that priority inheritance on counting semaphores should > > be > > > > explicitly enabled (disabled by default) and enabling it by default might > > > > lead to unexpected priority boost for low priority tasks that violates > > > > real-time requirements. > > > > > > > > It has been enabled by default since day one so nothing will be broken. > > > > No, all discussion and patch already show that it already breaks many > things and even crashes the system. To avoid the problem, we have to review > all places which use semaphore and make the adjustion if needed. Even five > years later, we still found the problem from ostest, the most used > verification tool on NuttX. > > > > Disabling it now will break things – that is an orthogonal discussion to > > spec compliance. > > > Yes, but "Strict POSIX compliance" is top one of "Inviolable Principles of > NuttX". Enabling priority inheritance by default breaks most programs which > use sem_t. > > > > Currently, priorioty inheritance is explicitly disabled > > in all places where it should not be enabled (i.e., signaling semahores). > > > > > Yes, but it's just for the mainline code. It isn't true for 3rd > party libraries even if they are compliant with POSIX strictly. > > > > > > > > If the default changes, then you would need to explicitly enable prority > > inheritance on all semaphores that are used as locks in order to retain > the > > previous behavior. See > > > > > https://cwiki.apache.org/confluence/display/NUTTX/Signaling+Semaphores+and+Priority+Inheritance > > > Yes, the plan is to fix all mainline code. Actually, we plan to use the > simple wrapper on top of sem: > https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/mutex.h > To make the lock usage inside the kernel more explicitly. > > > > > > – the POSIX definition does not have this issue because POSIX does not > > support priority inheritance on counting semaphores. > > > I initially expressed concerns but now I am convinced it should be fixed. We just need to make sure the docs are up to date and release notes are very clear what needs to change in non-mainline code that depends on the old non-compliant behavior. Cheers, Nathan