Zephyr: Event flags to signal tasks from ISR's

[lowlander]

Several RTOS like RTEMS and eCos have some sort of event flag set, mostly a 32bit int where one can set and wait for individual bits. A use case would be a ISR that gets called on RX done and on TX done (for example SPI). The ISR can than set bit 1 for RX done and bit 2 for TX done. The thread can than wait for bit 0 and bit 1 to be set. This is an easy API to do signaling from ISR. The only way I see how this can be done now is by using a semaphore and an int for the flags. But those flags will have to be volatile or atomic and when everybody has to implement it on its own which makes it probably error prone.

The RTEMS API; https://docs.rtems.org/branches/master/c-user/event/index.html and https://docs.rtems.org/doxygen/branches/master/group__ClassicEvent.html
The eCos API; https://doc.ecoscentric.com/ref/kernel-flags.html

[pabigot]

I've updated the RTEMS link above (the original was dead due to reorganization of the RTEMS project) and added one to the corresponding API.

I agree this is a large gap; if it were closed a new sensor API that doesn't hog threads could be constructed. The capabilities of the eCos and RTEMS solutions are not equal: some thought needs to go into selecting what features are most useful to Zephyr and how to integrate it into existing capabilities.

[nashif]

Here is the initial draft of the API

/**
 * @defgroup evgroup_apis Event Group APIs
 * @ingroup kernel_apis
 * @{
 */

enum {
    K_EVGROUP_NONE = 0,
    /** Match all flags. this is an AND operation */
    K_EVGROUP_ALL,
    /** Clear flags */
    K_EVGROUP_CLEAR
};

/**
 * @brief Initialize an event group.
 *
 * This routine initializes an event group object, prior to its first use.
 *
 * @param ev_flag Address of the event group.
 *
 * @return N/A
 */
__syscall void k_evgroup_init(struct k_evgroup *evgroup);

/**
 * @brief Set flags of an event group
 *
 *  This function sets the specified flags in the event group based on
 *  the set option specified.  All threads suspended on the group whose
 *  wait request can now be satisfied are resumed.
 *
 * @param evgroup Pointer to an event group
 * @param flags Flags to be set
 */
__syscall void k_evgroup_set(struct k_evgroup *evgroup, uint32_t flags);

/**
 * @brief Retrieve flags of an event flag group
 *
 * @param evgroup Pointer to an event group
 * @return flags The flags set in @a evgroup
 */
__syscall uint32_t k_evgroup_get(struct k_evgroup *evgroup);

/**
 * @brief  Clear flags of an event flag group
 *
 * @param evgroup Pointer to an event group
 * @param flags The flags to clear
 *
 */
__syscall void k_evgroup_clear(struct k_evgroup *evgroup, uint32_t flags);

/**
 * @brief Check if an event group has the requested flags.
 *
 * @note Can be called by ISRs, but @a timeout must be set to K_NO_WAIT.
 *
 * @param evgroup Pointer to an event group
 * @param flags Flags we should wait for
 * @param options Possible options are K_EVGROUP_ALL and/or K_EVGROUP_CLEAR.
 * @param timeout Non-negative waiting period to wait for operation to complete.
 *        Use K_NO_WAIT to return without waiting,
 *        or K_FOREVER to wait as long as necessary.
 * @return
 */
__syscall uint32_t k_evgroup_wait(struct k_evgroup *evgroup,
        uint32_t flags, uint8_t options, k_timeout_t timeout);

/**
 * @brief Statically define and initialize an event flag group.
 *
 * The event group can be accessed outside the module where it is
 * defined using:
 *
 * @code extern struct k_evgroup <name>; @endcode
 *
 * @param name Name of the semaphore.
 */
#define K_EVGROUP_DEFINE(name) \
    Z_STRUCT_SECTION_ITERABLE(k_evgroup, name) = \
        Z_EVGROUP_INITIALIZER(name);


/** @} */

[pabigot]

tl;dr: can we have a solution that can be waited for by k_poll?

While this does match the RTEMS and eCos capabilities it doesn't address a functionality gap in Zephyr that we really need to handle: the ability to wait for notifications from independent components.

For example the existing sensor API is difficult to use because it assumes each sensor has a dedicated thread to animate its functions, and/or that fetching samples is a synchronous operation that can put its caller to sleep. This is inconvenient for applications that monitor multiple sensors and don't want to burn memory on stack spaces for a large number of threads.

A new sensor API would ideally be animated by external threads, preferably work queues with different priorities and resources that could be selected on a per-sensor basis. Notification that a sample is ready, or that the sensor driver needs to be invoked from a thread to make progress, are provided nicely using an event system.

But what owns the event "group" instance? Having one per sensor is great: there will be a small number of common events at fixed positions, and individual drivers can add more for driver-specific events. But unless it's possible for a thread to block on multiple events e.g. via k_poll_event_group we haven't solved the problem of waiting threads on one sensor at a time.

An alternative architecture would use a single event group, but allocate individual flags to multiple devices. Management of this would be horrible, and the risk of setting/clearing flags that don't belong to the driver is high. There's also an artificial limit due to having only 32 flags available.

Another alternative is to keep the dedicated flag group per device but support a multi-level signal: when you set a flag in this event group, set this flag in the parent event group. Which would be safer but also pretty complex, and would increase either RAM or code size even if 32 flags is all the application or subsystem needs.

So it'd be really good if this had a way of waiting for notifications from any of an unbounded number of event groups.

[pabigot]

Regarding the as-proposed API, the reference implementations seem to behave this way, based on the documentation:

• eCos: wait until (all | any) specified events. Optionally clear the events before returning.
• RTEMs: wait until (all | any) specified events. Returned events are cleared unconditionally.

I would like to see Zephyr adopt the eCos capability: allow a thread to wait until multiple events are set simultaneously, or until at least one event is set. Also allow the caller to decide whether the set events waited for are cleared before the thread is woken, or left in place for the consumer to clear.

It looks like that's what the proposed API allows, assuming a little more detail on exactly what K_EVGROUP_CLEAR does, and its precise semantics when multiple threads are waiting on the same group with different options. (Assuming that's supported, which isn't specified. If only one thread can wait on each group that's a little less useful.)

[nashif]

tl;dr: can we have a solution that can be waited for by k_poll?

not planned with this initial proposal, the API above is going to implement the "textbook" definition of event flags that is available almost with the same semantics and APIs in every RTOS out there, not only the two mentioned here for reference. Threadx, uC, Freertos all have this with similar and basic APIs as being proposed here with minor implementation changes.

Initial goal here is to introduce a familiar API that would allow applications using event flags in other RTOSes to be ported without too much pain.

I would like to see Zephyr adopt the eCos capability: allow a thread to wait until multiple events are set simultaneously, or until at least one event is set.

This is what the above API does. A thread may expect either all flags in a set to have occurred (AND), or any flag in a set (OR).

Also allow the caller to decide whether the set events waited for are cleared before the thread is woken, or left in place for the consumer to clear.

The call of k_evgroup_wait can set this as an option, i.e. the calling thread can clear the flags and thus block other threads in the queue from unpending.

I just read the eCos link above, and the implementation is going to be similar.

If only one thread can wait on each group that's a little less useful.

No, multiple threads can wait on the same group, though they can block on different flags that could be fulfilled by the group. If a thread clears the flags, the operation is aborted and all remaining threads stay blocked.

I will provide some more details here about the theory of operation ti demystify this.

[andrewboie]

tl;dr: can we have a solution that can be waited for by k_poll?

not planned with this initial proposal, the API above is going to implement the "textbook" definition of event flags that is available almost with the same semantics and APIs in every RTOS out there, not only the two mentioned here for reference. Threadx, uC, Freertos all have this with similar and basic APIs as being proposed here with minor implementation changes.

Follow-up question: can this supersede k_poll?

[nashif]

Follow-up question: can this supersede k_poll?

I think it could, though we have a few heavy users of k_poll in the tree so we need to see how easily it would be to convert them to event groups

[andrewboie]

Since this is a new design, I would suggest having this cover all k_poll uses as well. Then once we have it in place we can start the deprecation process for k_poll. We've been wanting a better way of waiting for multiple events on one thread for some time now and I think this is a good opportunity.

[nashif]

so on the surface and without going too much into details of k_poll and if we use event flags as is, the main change would be the fact that events are signaled from the application instead of in the object implementation (semaphore and fifo now) and this actually allow to have events almost on anything and not be limited to what k_poll supports (object events). K_POLL_TYPE_SIGNAL can be directly implemented with event groups and will be able to support multiple threads waiting on an event.

I took some existing code (not complex) and tried to convert it, see this for example (did not run the code, just built it):

diff --git a/subsys/canbus/isotp/isotp.c b/subsys/canbus/isotp/isotp.c
index 7515067714..237cfff10e 100644
--- a/subsys/canbus/isotp/isotp.c
+++ b/subsys/canbus/isotp/isotp.c
@@ -44,6 +44,14 @@ static struct k_work_q isotp_workq;

 static void receive_state_machine(struct isotp_recv_ctx *ctx);

+K_EVGROUP_DEFINE(evgrp)
+#define DATA_AVAILABLE ( 1 << 0 )
+
+static void fifo_put(struct k_fifo *fifo, struct net_buf *buf) {
+       net_buf_put(fifo, buf);
+       k_evgroup_set(&evgrp, DATA_AVAILABLE);
+}
+
 /*
  * Wake every context that is waiting for a buffer
  */
@@ -73,17 +81,6 @@ static void receive_ff_sf_pool_free(struct net_buf *buf)
        }
 }

-static inline int _k_fifo_wait_non_empty(struct k_fifo *fifo,
-                                        k_timeout_t timeout)
-{
-       struct k_poll_event events[] = {
-               K_POLL_EVENT_INITIALIZER(K_POLL_TYPE_FIFO_DATA_AVAILABLE,
-                                        K_POLL_MODE_NOTIFY_ONLY, fifo),
-       };
-
-       return k_poll(events, ARRAY_SIZE(events), timeout);
-}
-
 static inline void receive_report_error(struct isotp_recv_ctx *ctx, int err)
 {
        ctx->state = ISOTP_RX_STATE_ERR;
@@ -268,7 +265,7 @@ static void receive_state_machine(struct isotp_recv_ctx *ctx)
                ud_rem_len = net_buf_user_data(ctx->buf);
                *ud_rem_len = 0;
                LOG_DBG("SM process SF of length %d", ctx->buf->len);
-               net_buf_put(&ctx->fifo, ctx->buf);
+               fifo_put(&ctx->fifo, ctx->buf);
                ctx->state = ISOTP_RX_STATE_RECYCLE;
                receive_state_machine(ctx);
                break;
@@ -293,7 +290,7 @@ static void receive_state_machine(struct isotp_recv_ctx *ctx)
                        ctx->bs = ctx->opts.bs;
                        ud_rem_len = net_buf_user_data(ctx->buf);
                        *ud_rem_len = ctx->length;
-                       net_buf_put(&ctx->fifo, ctx->buf);
+                       fifo_put(&ctx->fifo, ctx->buf);
                }

                ctx->wft = ISOTP_WFT_FIRST;
@@ -483,7 +480,7 @@ static void process_cf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
        if (ctx->length == 0) {
                ctx->state = ISOTP_RX_STATE_RECYCLE;
                *ud_rem_len = 0;
-               net_buf_put(&ctx->fifo, ctx->buf);
+               fifo_put(&ctx->fifo, ctx->buf);
                return;
        }

@@ -491,7 +488,7 @@ static void process_cf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
                LOG_DBG("Block is complete. Allocate new buffer");
                ctx->bs = ctx->opts.bs;
                *ud_rem_len = ctx->length;
-               net_buf_put(&ctx->fifo, ctx->buf);
+               fifo_put(&ctx->fifo, ctx->buf);
                ctx->state = ISOTP_RX_STATE_TRY_ALLOC;
        }
 }
@@ -673,19 +670,17 @@ int isotp_recv(struct isotp_recv_ctx *ctx, uint8_t *data, size_t len,
        struct net_buf *buf;
        int ret;

-       ret = _k_fifo_wait_non_empty(&ctx->fifo, timeout);
-       if (ret) {
+       ret = k_evgroup_wait(&evgrp, DATA_AVAILABLE, K_EVGROUP_CLEAR, timeout);
+       if ( (ret & DATA_AVAILABLE) != 0) {
                if (ctx->error_nr) {
                        ret = ctx->error_nr;
                        ctx->error_nr = 0;
                        return ret;
                }

-               if (ret == -EAGAIN) {
-                       return ISOTP_RECV_TIMEOUT;
-               }
-
                return ISOTP_N_ERROR;
+       } else {
+               return ISOTP_RECV_TIMEOUT;
        }

        buf = k_fifo_peek_head(&ctx->fifo);

edit: the return code checking for k_evgroup_wait above is wrong, but you get the point...

[pabigot]

K_POLL_TYPE_SIGNAL can be directly implemented with event groups and will be able to support multiple threads waiting on an event.

Can you provide more detail on how this will work? k_poll_signal() is a foundational capability for asynchronous operations, providing the operation's result via sig.result, and I don't see how you can get an integral operation result out of an event group.

[nashif]

I am talking about this use case (from the docs), have not used k_poll APIs extensively, so I might be missing something.

struct k_poll_signal signal;

// thread A
void do_stuff(void)
{
    k_poll_signal_init(&signal);

    struct k_poll_event events[1] = {
        K_POLL_EVENT_INITIALIZER(K_POLL_TYPE_SIGNAL,
                                 K_POLL_MODE_NOTIFY_ONLY,
                                 &signal),
    };

    k_poll(events, 1, K_FOREVER);

    if (events.signal->result == 0x1337) {
        // A-OK!
    } else {
        // weird error
    }
}

// thread B
void signal_do_stuff(void)
{
    k_poll_signal_raise(&signal, 0x1337);
}

which can be done with flags using:

K_EVGROUP_DEFINE(evg)
#define SOME_EVENT BIT(1)

// thread A
void do_stuff(void)
{
    int options;
    int ret;

    ret = k_evgroup_wait(&evg, SOME_EVENT , options, K_FOREVER);

    // ret holds the event that unpended the thread
    if ( (ret & SOME_EVENT != 0)  ) {
        // A-OK!
    } else {
        // weird error
    }
}

// thread B
void signal_do_stuff(void)
{
    k_evgroup_set(&signal, SOME_EVENT);
}

[pabigot]

I am talking about this use case (from the docs), have not used k_poll APIs extensively, so I might be missing something.

Yes, you can get a binary success/failure. But you can't get the detailed "failed with -EIO" or "failed with -ENOTSUP" or "succeeded with 3 frames transferred" that are available with the current API.

See spi_context_complete or any of the components that use asynchronous notification with signal notification (which until #25950 gets reprioritized is the only way user mode can do asynchronous operations that include a non-binary result).

If the internals of kernel polling are replaced I'd probably be OK with it, but a poll API is a nice, well understood solution to multiplexing notifications. It doesn't have an artificial limit of 32 events, nor the complexity of assigning flag numbers to unrelated functions that I raised in https://github.com/zephyrproject-rtos/zephyr/issues/27868#issuecomment-696789879.