Fwd: [PATCH 1/2] score: Add and use Watchdog_Clock for clock types.

[Sebastian Huber]

On 16/06/16 08:09, Sebastian Huber wrote:
>> diff --git a/cpukit/score/include/rtems/score/threadqimpl.h
>> b/cpukit/score/include/rtems/score/threadqimpl.h
>> index 73d4de2..f59a334 100644
>> --- a/cpukit/score/include/rtems/score/threadqimpl.h
>> +++ b/cpukit/score/include/rtems/score/threadqimpl.h
>> @@ -349,6 +349,7 @@ Thread_Control *_Thread_queue_Do_dequeue(
>>    *       executing,
>>    *       STATES_WAITING_FOR_MUTEX,
>>    *       WATCHDOG_NO_TIMEOUT,
>> + *       WATCHDOG_RELATIVE,
>>    *       0,
>>    *       &queue_context
>>    *     );
>> @@ -362,6 +363,7 @@ Thread_Control *_Thread_queue_Do_dequeue(
>>    * @param[in] state The new state of the thread.
>>    * @param[in] timeout Interval to wait.  Use WATCHDOG_NO_TIMEOUT to 
>> block
>>    * potentially forever.
>> + * @param[in] clock The kind of clock used for the interval.
>>    * @param[in] queue_context The thread queue context of the lock 
>> acquire.
>>    */
>>   void _Thread_queue_Enqueue_critical(
>> @@ -370,6 +372,7 @@ void _Thread_queue_Enqueue_critical(
>>     Thread_Control                *the_thread,
>>     States_Control                 state,
>>     Watchdog_Interval              timeout,
>> +  Watchdog_Clock                 clock,
>>     Thread_queue_Context          *queue_context
>>   );
>
> We already have a lot of parameters for the 
> _Thread_queue_Enqueue_critical(). I would rather move the timeout 
> parameters to the queue_context, since this is more efficient.
>
> The Watchdog_Interval is 32-bit, thus not suitable for the absolute 
> timeouts.
>
> A value of 0 is valid for absolute timeouts, see what happens if you 
> get that wrong
>
> https://www.wired.com/2016/02/dont-set-your-iphone-back-to-1970-no-matter-what/ 
>
>
> So, I would move the WATCHDOG_NO_TIMEOUT to your new Watchdog_Clock.
>
> [...] 

 From IRC log:

[19:44] <gedare> sebhub: Does it make sense to you to redefine 
Watchdog_Interval as a struct containing a union for the interval and an 
enum for the Clock "Discipline" (relative/absolute)?
[19:45] <gedare> ^the union for uint32_t relative, uint64_t absolute--or 
else we make all intervals 64-bit

No, I would not change Watchdog_Interval. We should keep the ticks based 
services as is. The problem is that _Thread_queue_Enqueue_critical() 
doesn't support absolute timeouts currently. I would do something like 
the attached.

We have

void _Thread_queue_Enqueue_critical(
   Thread_queue_Queue            *queue,
   const Thread_queue_Operations *operations,
   Thread_Control                *the_thread,
   States_Control                 state,
   Thread_queue_Context          *queue_context
)
{
   Per_CPU_Control *cpu_self;
   bool             success;

#if defined(RTEMS_MULTIPROCESSING)
   if ( _Thread_MP_Is_receive( the_thread ) && 
the_thread->receive_packet ) {
     the_thread = _Thread_MP_Allocate_proxy( state );
   }
#endif

   _Thread_Lock_set( the_thread, &queue->Lock );

   the_thread->Wait.return_code = STATUS_SUCCESSFUL;
   _Thread_Wait_set_queue( the_thread, queue );
   _Thread_Wait_set_operations( the_thread, operations );

   ( *operations->enqueue )( queue, the_thread );

Thus, we would have to store the timeout related parameters to the stack 
anyway, since ( *operations->enqueue )( queue, the_thread ) is a 
function call which cannot be optimized way (except maybe through 
link-time optimization). So, there is no overhead due to the use of the 
queue context.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-score-_Thread_queue_Enqueue_critical.patch
Type: text/x-patch
Size: 6060 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20160621/02cafe35/attachment-0002.bin>