On 2/21/19 7:42 PM, Heinz Junkes wrote:
> Yes, I can confirm that with your changes the behavior has now changed and everything works even with the _main_ task
> with prio 0. (Tested with RTEMS5)
>
> I also already  removed the test  for isEpicsThread in
> epicsThreadSetPriority
Just for the record: IIRC, this is the only place where 'isEpicsThread' 
is used. If the
test is removed then we should remove the struct member, too, IMHO.

- Till
>
>   if(!pthreadInfo->isEpicsThread) {
>>          fprintf(stderr,"epicsThreadSetPriority called by non epics
>> thread\n");
>>          return;
>>      }
> in rtems_init.c I set the priority in the epicsThread-Structure:
>
> ...
>      if (epicsRtemsInitPostSetBootConfigFromNVRAM(&rtems_bsdnet_config) != 0)
>          delayedPanic("epicsRtemsInitPostSetBootConfigFromNVRAM");
>
>      #if __RTEMS_MAJOR__>4
>
>      /*
>       * Override RTEMS Posix configuration, as it get started with posix prio 2
>       */
>
>      epicsThreadSetPriority(epicsThreadGetIdSelf(),
> epicsThreadPriorityIocsh);
>
> #else
>       /*
>       * Override RTEMS configuration
>       */
>      rtems_task_set_priority (
>                       RTEMS_SELF,
>                       epicsThreadGetOssPriorityValue(epicsThreadPriorityIocsh),
>                       &newpri);
> #endif
>
>      /*
>       * Create a reasonable environment
>       */
>      initConsole ();
> …
>
> Works great for RTEMS5. Thanks for your changes. I've been sitting there for a long time.
> Heinz
>
>
>> On 21. Feb 2019, at 19:25, Till Straumann <email address hidden> wrote:
>>
>> With the changes I made, epicsThreadSetPriority() should -- in principle
>> -- also work for non-EPICS threads,
>> as long as the target thread uses the same scheduling policy as pcommonAttr.
>>
>> However, for sake of backwards compatibility I did not yet remove the
>> test
>>
>>      if(!pthreadInfo->isEpicsThread) {
>>          fprintf(stderr,"epicsThreadSetPriority called by non epics
>> thread\n");
>>          return;
>>      }
>>
>> (but you can find the comment:
>>
>>      /* T.S.: There is in principle no problem with changing the priority of
>>       * a non-EPICS thread -- as long as it uses the same scheduling policy.
>>       * For sake of backwards compatibility I leave this test for now but
>>       * it could as well be removed (and the 'isEpicsThread' member as
>> well).
>>       */
>> ).
>>
>> I.e., as a result of this discussion we could remove the above test and
>> you then should be
>> able to set the priority of the 'main' thread.
>>
>> More comments in-line below...
>>
>> On 2/21/19 9:33 AM, Heinz Junkes wrote:
>>> Thanks Till,
>>>
>>> I'll apply that for my RTESM5 port too and test it.
>>> (Meanwhile I use the posix osdTthread variant also for RTEMS5).
>>>
>>> If "we" are already there, the following call has been causing problems
>>> for a long time.
>>>
>>> -    pthreadInfo = init_threadInfo("_main_",0,epicsThreadGetStackSize(epicsThreadStackSmall),0,0);
>>> +    pthreadInfo = init_threadInfo("_main_",0,0,0);
>>>
>>> So that the "ur-task" is started, during POSIX_Init. But as non Epics-task it gets the OSI-prio "0".
>>> Shouldn't something like IocShPrio 91 be set here?
>>>
>>> Unfortunately I can't set it in rtems_init with epicsThreadSetPriority because it is a
>>> non-epics-task.
>>>
>>> But in some tests we read the epicsPrio from this thread (and you get it back as 0) and then e.g. in the ringPointerTest.c we create some threads with the prio 0 and 1 ;-)
>>> Then they hang ...
>> Sure, the fallback to '0' works under linux where the main thread uses
>> SCHED_OTHER. Under RTEMS, however,
>> the 'main' thread is IIRC a 'normal' SCHED_FIFO thread which has a
>> priority higher than 0 and thus the true
>> ordering of priorities is different from what it seems.
>>
>> The aforementioned changes may allow you to fix this, but IMHO,
>> regardless or any decision (re. removal of the 'if ! isEpicsThread')
>> it might be better to change the test so that it uses epicsThreads only
>> (by spawning separate threads for producer and consumer) because
>> ultimately that is the only guarantee that the two threads use the same
>> scheduling policy (in a portable way).
>>
>> - Till
>>> "ringPointerTest.c" line 171 of 248
>>>
>>> static void testPair(int locked)
>>> {
>>>      unsigned int myprio = epicsThreadGetPrioritySelf(), consumerprio;
>>>      pairPvt pvt;
>>>
>>> ...
>>> testDiag("single producer, single consumer with%s locking", locked?"":"out");
>>>
>>>      /* give the consumer thread a slightly higher priority so that
>>>       * it can preempt us on RTOS targets.  On non-RTOS targets
>>>       * we expect to be preempted at some random time
>>>       */
>>>      if(!epicsThreadLowestPriorityLevelAbove(myprio, &consumerprio))
>>>          testAbort("Can't run test from thread with highest priority");
>>>
>>>      epicsThreadMustCreate("pair", consumerprio,
>>>                            epicsThreadGetStackSize(epicsThreadStackSmall),
>>>                            &pairConsumer, &pvt);
>>>      /* wait for worker to start */
>>>      epicsEventMustWait(pvt.sync);
>>>
>> -- 
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/1816841
>>
>> Title:
>>   non-epicsThreads may receive wrong priority and crash
>>
>> Status in EPICS Base:
>>   New
>>
>> Bug description:
>>   The following applies to the POSIX threading implementation.
>>
>>   If an application is launched with enough privilege to use the real-
>>   time scheduler (SCHED_FIFO) and creates some non-epicsThreads which
>>   use SCHED_OTHER then a bug is triggered when the non-epicsThread gets
>>   a 'pseudo-epics context' attached (via 'createImplicit()' e.g, by
>>   calling epicsThreadSuspendSelf()):
>>
>>   createImplicit() computes a thread priority for the non-epics thread
>>   assuming SCHED_FIFO -- which is actually wrong. Under some
>>   circumstances the 'illegal' priority value can lead to a crash.
>>
>>   The attached patch fixes this and cleans up some cruft. It also makes
>>   sure that whenever 'pcommonAttr' is used the scheduling policy of
>>   pcommonAttr matches the scheduling policy of the target thread.
>>
>>   The patch also adds a test program which exercises the described
>>   behavior - however, it is necessary to execute the test with
>>   sufficient privilege for epicsThreads to use SCHED_FIFO (if the
>>   privilege is lacking then the test is skipped).
>>
>>   The patch is against R7.0.2.
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/epics-base/+bug/1816841/+subscriptions