On Fri, Mar 31, 2023 at 12:23 AM Gregory Nutt wrote: > > > In his Confluence paper on "Signaling Semaphores and Priority > Inheritance”, Brennan Ashton’s analysis is both thorough and accurate; ... > > Minor fix. I wrote the paper, Brennan converted the Confluence page > from an older DocuWiki page
Respect :-) > > The solution Brennan suggests is to initialize semaphores used as > signaling events as follows: > > sem_init(&sem, 0, 0); > > sem_setprotocol(&sem, SEM_PRIO_NONE); > > > > this is, of course, correct, but retains the dual purpose of > sem_wait() and sem_post(). I believe this can be confusing and will > continue to be a source of subtle errors. I suggest going a step > further and isolate the two use cases. Let the current sem_init, > sem_wait, sem_post be used only for the resource locking use case. > > > > For the signaling use case, create a new API for event signaling > within the kernel: nxev_init, nxev_wait, nxev_post where: nxev_init is > simply: > > sem_init(&nxev, 0, 0); > > sem_setprotocol(&nxev, SEM_PRIO_NONE); > > > > and: > > #define nxev_wait sem_wait > > #define nxev_post sem_post > > > > In the case were PRIORITY_INHERITANCE is not configured, > sem_setprotocol() does nothing and the nxev_*() API is still used for > event notification. > > > > This may seem a trivial change, but having specific API function > names for the two specific use cases would, I believe, all but eliminate > future confusion; especially given that most people look to existing > drivers to use as a template. Finally, enabling or disabling > PRIORITY_INHERITANCE would not introduce the subtle error Brennan > documented. > > Your suggestion would clarify the usage. > > I was thinking out a conceptually simple solution that should also > resolve the risk in usage: Just change the default state to > SEM_PRIO_NONE for all semaphores. That would make the default protocol > for semaphore be no priority inheritance. > > This would be a lot of work, however. All occurrences of sem_init() > would have to be examined: > > For the signaling use case, you would do nothing. We would have to > remove all sem_setprotocol(&nxev, SEM_PRIO_NONE); > For the resource locking use case, you would have to add > sem_setprotocol(&nxev, SEM_PRIO_INHERIT); assuming priority inheritance > is enabled. > > The eliminates the risk of inappropriately using priority inheritance on > a locking semaphore. The consequences of doing that are very bad: > https://nuttx.yahoogroups.narkive.com/3hggphCi/problem-related-semaphore-and-priority-inheritance > > Then the only error that the user can make is to fail to select priority > inheritance when it would do good. That is a lesser error and, as you > note, usually OK. +1 :-) -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
