Experimental Physics and
Industrial Control System

mdavidsaver via Core-talk <core-talk at aps.anl.gov> · Fri, 18 Dec 2020 17:17:10 -0000

Review: Needs Fixing

I pushed your latest to github for testing.  There are a couple of failures with RTEMS and OSX.

https://github.com/mdavidsaver/epics-base/actions/runs/430893895

I still think it is necessary for the new globalAttrInit() to probe for runtime support of PTHREAD_PRIO_INHERIT.  I've included a sketch of this as a diff comment.

Unless I'm misreading, it looks like only the recursive implementation would have PTHREAD_PRIO_INHERIT set.  So the mutex used internally in osdThread and osdSpin would continue to be non-PI.

Diff comments:

> diff --git a/modules/libcom/src/osi/os/posix/osdMutex.c b/modules/libcom/src/osi/os/posix/osdMutex.c
> index 707ae63..a104229 100644
> --- a/modules/libcom/src/osi/os/posix/osdMutex.c
> +++ b/modules/libcom/src/osi/os/posix/osdMutex.c
> @@ -38,6 +39,53 @@
>          cantProceed((method)); \
>      }
>  
> +#if defined(DONT_USE_POSIX_THREAD_PRIORITY_SCHEDULING)
> +#undef _POSIX_THREAD_PRIO_INHERIT
> +#endif
> +
> +/* Global var - pthread_once does not support passing args but it is more efficient
> + * than epicsThreadOnce which always acquires a mutex.
> + */
> +static pthread_mutexattr_t globalAttrRecursive;
> +static pthread_once_t      globalAttrInitOnce = PTHREAD_ONCE_INIT;
> +
> +static void globalAttrInit()
> +{
> +    int status;
> +
> +    status = pthread_mutexattr_init(&globalAttrRecursive);
> +    checkStatusQuit(status,"pthread_mutexattr_init","globalAttrInit");
> +
> +#if defined _POSIX_THREAD_PRIO_INHERIT
> +    status = pthread_mutexattr_setprotocol(&globalAttrRecursive, PTHREAD_PRIO_INHERIT);
> +    if (errVerbose) checkStatus(status, "pthread_mutexattr_setprotocol(PTHREAD_PRIO_INHERIT)");
> +#endif
> +
> +    status = pthread_mutexattr_settype(&globalAttrRecursive, PTHREAD_MUTEX_RECURSIVE);
> +    checkStatusQuit(status, "pthread_mutexattr_settype(PTHREAD_MUTEX_RECURSIVE)", "globalAttrInit");

What I'd like to see here would look something like.

status = pthread_mutex_init(&temp, &globalAttrRecursive);
if(status == EOPNOTSUPP) {
    pthread_mutexattr_setprotocol(&globalAttrRecursive, PTHREAD_PRIO_NONE);
}

> +}
> +
> +int osdPosixMutexInit (pthread_mutex_t *m, int mutextype)
> +{
> +    pthread_mutexattr_t *atts;
> +    int status;
> +
> +    status = pthread_once( &globalAttrInitOnce, globalAttrInit );
> +    checkStatusQuit(status,"pthread_once","epicsPosixMutexAttrGet");
> +
> +    switch (mutextype) {
> +        case PTHREAD_MUTEX_DEFAULT:
> +            atts = NULL;
> +            break;
> +        case PTHREAD_MUTEX_RECURSIVE:
> +            atts = &globalAttrRecursive;
> +            break;
> +        default:
> +            return ENOTSUP;
> +    }
> +    return pthread_mutex_init(m, atts);
> +}
> +
>  static int mutexLock(pthread_mutex_t *id)
>  {
>      int status;

-- 
https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+merge/394327
Your team EPICS Core Developers is subscribed to branch epics-base:7.0.

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System