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

Reply via email to