RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.94k stars 1.99k forks source link

Support for Condition Variables #727

Closed Neverlord closed 10 years ago

Neverlord commented 10 years ago

Condition Variables are a very basic and commonly used synchronization primitive - plus, they cannot be implemented in userspace properly. Hence, it would make sense to implement it in RIOT directly, maybe using an API like this:

`(I) cv_notify_one(...) // notifies one waiting thread (II) cv_notify_all(...) // notifies all waiting threads (III) cv_wait(...) // blocks the caller until it is notified by another thread (IV) cv_wait_for(...) // blocks the caller until it is notified or until the specified timeout duration is reached (V) cv_wait_until(...) // blocks the caller until it is notified or until the specified time point has been reached``

The difference between (IV) and (V) is that the first one uses a relative timeout whereas the latter uses an absolute timeout. Function (V) is important for dealing with the problem of spurious wakeup. Function (IV) is merely a convenience function and can be implemented using (V).

All wait functions take a mutex that must be locked when calling the wait function. After the thread has been woken up again, the mutex is locked again. CVs are also part of the POSIX API (pthread_cond_broadcast, pthread_cond_destroy, pthread_cond_init, pthread_cond_signal, pthread_cond_timedwait and pthread_cond_wait), so adding CVs would also boost RIOTs POSIX compatibility.

kaspar030 commented 10 years ago

+1 for implementing these in RIOT. Any reason for not just using the pthread_ names? The API seems quite reduced. Also, could you elaborate on "they cannot be implemented in userspace properly"?

waehlisch commented 10 years ago

@kaspar030, Dominik wrote in his original email to the devel list that the required locking "cannot be guaranteed in userspace unless RIOT would offer a function like "mutex_unlock_and_sleep" which atomically unlocks the mutex and puts the calling thread to sleep mode."

OlegHahm commented 10 years ago

@BytesGalore, @haukepetersen and me had some discussion about this topic today. The conclusion was the we couldn't find another solution than implementing this mutex_unlock_and_sleep() (probably as part of the mutex module) and then implementing the suggested CV API somewhere in sys.

kaspar030 commented 10 years ago

@waehlisch Thx, I missed that.

@all: Would that be more difficult than

mutex_unlock_and_sleep() {
    dINT();
    sched_set_status(current_thread, STATUS_SLEEPING);
    mutex_unlock(mutex)
    eINT();
    thread_yield();
}

?

BytesGalore commented 10 years ago

no, it is exactly what we had in mind

Neverlord commented 10 years ago

@kaspar030 just implementing the POSIX API directly would be perfectly fine. I have just assumed that you want a separate RIOT API with a wrapper for POSIX compatibility - in the same way threading has its own API on RIOT. At the end of the day, we need the POSIX interface anyways.

OlegHahm commented 10 years ago

I think we should have POSIX compatibility wrappers only where we can provide a simpler implementation without POSIX API. For condition variables I don't see this subset of functionality right now.

BytesGalore commented 10 years ago

The only that might be obsolete in the implementation are the condition attributes passed to and used in the init function. The POSIX implementation would be similar to https://github.com/BytesGalore/RIOT/commit/9a25eabcdc2087e0c1c6218db591dd2d483183b2 (its a stub atm.)

OlegHahm commented 10 years ago

Will be closed by https://github.com/RIOT-OS/RIOT/pull/771

BytesGalore commented 10 years ago

closing due to #771