[ECOS] RE: FW: Counting and Binary semaphore, with timeout

["Devaughn, Hans" <hans dot devaughn at siemens dot com>]

David,

Thank you for your speedy reply.

I wasn't question the philosophy behind the implementation. I was having
trouble getting the thread to go blocked on a timed wait. The key to my
problem was your last sentence. I wasn't using the absolute time and the
code thought the time had already elapsed.

Thanks for the help.

Hans

-----Original Message-----
From: David Brennan [mailto:eCos@brennanhome.com] 
Sent: Friday, October 29, 2004 9:02 AM
To: Devaughn, Hans; eCos Discussion List
Subject: Re: FW: Counting and Binary semaphore, with timeout


It is always best to keep these discussions on the list.

Devaughn, Hans wrote:

>David,
>
>I just discovered that you wrote the original patch for the binary 
>semaphore. Could you shed some light on the problem I submitted to the 
>discussion group?
>
>Thanks in advance.
>
>Hans
>
>-----Original Message-----
>From: Devaughn, Hans
>Sent: Thursday, October 28, 2004 5:06 PM
>To: 'ecos-discuss@sources.redhat.com'
>Cc: El Houmaidi, Mounire
>Subject: Counting and Binary semaphore, with timeout
>
>
>I am having a problem getting the counting and binary semphores, with 
>timeout, to work for the following reason
>
>The following code executes (in xxx::wait) (from sem_binary.cxx)
>    // Set the timer _once_ outside the loop.
>    self->set_timer( timeout, Cyg_Thread::TIMEOUT  );
>
>The inline code (in thread.inl) executes the set_timer
>inline void Cyg_Thread::set_timer(
>    cyg_tick_count      trigger,
>    cyg_reason          reason
>)
>{
>#ifdef CYGFUN_KERNEL_THREADS_TIMER
>    self()->sleep_reason = reason;
>    self()->wake_reason = NONE;
>    self()->timer.initialize( trigger);
>#endif
>}
>
>The sleep_reason is set to TIMEOUT (as passed as the reason) in the 1st 
>executable line The wake_reason is set to NONE in the 2nd executable 
>line The call to timer.initialize calls add_alarm (in clock.cxx)
>
>The following line (in add_alarm) calls alarm (in thread.cxx)
>        alarm->alarm(alarm, alarm->data);
>
>  
>
If you look closely, this only gets called if your alarm is set in the past.
    // Check here for an alarm that triggers now or in the past and
    // call its alarm function immediately.
    if( alarm->trigger <= counter )
    {
        CYG_INSTRUMENT_ALARM( CALL, this, alarm );

        // call alarm function. Note that this is being
        // called here before the add_alarm has returned.
        // Note that this function may disable the alarm.
       
        alarm->alarm(alarm, alarm->data);
If the alarm did already happen, then the desired operation of the timed 
wait is for the it to not block at all. (That was existing.) My change 
was to allow taking the semaphore if the timeout happened in the past if 
the semaphore is available to be taken. Is that your problem?

>The following code (in thrad.cxx) causes the wake reason to be set to 
>TIMEOUT because the sleep_reason was TIMEOUT
>    Cyg_Thread::cyg_reason sleep_reason = thread->get_sleep_reason();
>    
>    switch( sleep_reason ) {
>        
>    case Cyg_Thread::DESTRUCT:
>    case Cyg_Thread::BREAK:
>    case Cyg_Thread::EXIT:
>    case Cyg_Thread::NONE:
>    case Cyg_Thread::WAIT:
>    case Cyg_Thread::DONE:
>        // Do nothing in any of these cases. Most are here to
>        // keep the compiler happy.
>        Cyg_Scheduler::unlock();
>        CYG_REPORT_RETURN();
>        return;
>
>    case Cyg_Thread::DELAY:
>        // The thread was simply delaying, unless it has been
>        // woken up for some other reason, wake it now.
>        thread->set_wake_reason(Cyg_Thread::DONE);
>        break;
>
>    case Cyg_Thread::TIMEOUT:
>        // The thread has timed out, set the wake reason to
>        // TIMEOUT and restart.
>        thread->set_wake_reason(Cyg_Thread::TIMEOUT);
>        break;
>
>When we get back to the semaphore code the following check is made 
>(these line are from bin_sem.cxx)
>    if( self->get_wake_reason() != Cyg_Thread::NONE && !state )
>        result = false;
>
>The wake_reason is TIMEOUT, so the first test is always true. I 
>initialized state=0 (I initialized counter in counting semaphore = 0), 
>therefore "result" is set to false.
>
If state is set to 0, then the semaphore is not available, and the alarm 
occurred in the past, the code should not block.

>Then, when the next statement is executed
>    while ( !state && result ) {
>
>the thread will not be blocked.
>
>
>  
>
There was some discussion on the eCos discuss list when I wrote this 
patch about the proper operation in this situation. If you feel it is 
incorrect feel free to explain why. Here is my reason for wanting it to 
work this way.

Our application calculates the time-out value on every wait, it is 
possible to calculate a timeout value of (relative) 0, which could be 
checked for prior to doing the timed wait, and doing a trywait instead. 
But there is always the possibility of a race condition with the clock 
tick in that scenario. It makes much more sense for the semaphore to 
still be available if it really is available even if the timeout period 
has expired.

By the way, make sure you use absolute time for your timeout value.

David Brennan

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss