Code review comment for ~dirk.zimoch/epics-base:epicsMutexPriorityInheritance

> Names beginning with epics are generally intended for public use, which the
> epicsPosixMutex stuff shouldn't be. If I've understood the code correctly you
> aren't replacing the pthread_mutex_t type but are providing a wrapper for
> pthread_mutex_init() that creates one, and you're also moving the need for the
> caller to set mutex attributes on the new mutex. Any new publicly visible
> symbols that are meant to be internal to a libCom implementation should begin
> with "osd", so maybe osd_mutex_init()?

I think something with "posix" in the name would be good as it only affects posix mutexes?
But maybe Till had more in mind: An OSI interface to ask for recursive or (maybe cheaper) non-recursive mutexes, no matter how they are implemented and what the OS is. Could be semM vs semB in vxworks or whatever in Windows. Of course the header should not be "osd" in that case.
Is there a need to ask for (possibly cheaper) non-recursive mutexes? They are now only used in osdSpin.c (only if no posix spin locks are available), in osdEvent.c and osdThread.c. Everything else uses recursive mutexes.

>
> I've read the essay in the comments just below epicsMutexOsdShow(), but I'm
> not sure how it helps future maintainers to understand this file. The
> discussion there doesn't seem appropriate in source code comments.

I will remove the essay.

>
> Is the new test program generic? It has some CPU-affinity stuff in it which is
> conditional on glibc; what happens on other OSs (Windows, MacOS, RTEMS-5,
> embedded Linux) that can also do SMP and don't use glibc?

The change is in posix only, not Windows or RTEMS. The test is Linux only. Maybe it should be posix only as well. The CPU affinities are used to guarantee CPU starvarion of the high priority thread on multi-core systems as well. Otherwise the test may show a false success. Should be possible to run the test on all posix architectures.

Do we need a cpu affinity OSI API?

>
> I'm not sure I like the new environment variable or understand what effect not
> defining it will have. Is there a good reason to continue to support the
> previous mode if in practice it's broken? As long as we document the changes
> in behavior when SCHED_FIFO is available (I'm assuming nothing should change
> when it isn't) I'm not convinced we need the env switch, which would resolve
> Dirk's question about how to set an environment variable before running a test
> program.

The previous implementation is a bit "faster on average" because the mutexes are simpler. But "fast" only counts in RT context, and there "faster on average" basically means "wrong".

I can take the environment variable out. It was in so we can easily switch back and forth to see the difference. And as an anti change-panic device.

>
> However I wonder if it would be desirable to run this new test (and maybe some
> of the other thread-related test programs too) twice, once with the default
> scheduler and once under SCHED_FIFO?

You mean expected failure is success? How to program that?