From b289fd1420010119ad47d74d7359bffffe19a56c Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Tue, 28 Feb 2023 15:06:48 +0100 Subject: [PATCH] MINOR: event_hdl: normal tasks support for advanced async mode advanced async mode (EVENT_HDL_ASYNC_TASK) provided full support for custom tasklets registration. Due to the similarities between tasks and tasklets, it may be useful to use the advanced mode with an existing task (not a tasklet). While the API did not explicitly disallow this usage, things would get bad if we try to wakeup a task using tasklet_wakeup() for notifying the task about new events. To make the API support both custom tasks and tasklets, we use the TASK_IS_TASKLET() macro to call the proper waking function depending on the task's type: - For tasklets: we use tasklet_wakeup() - For tasks: we use task_wakeup() If 68e692da0 ("MINOR: event_hdl: add event handler base api") is being backported, then this commit should be backported with it. --- doc/internals/api/event_hdl.txt | 12 ++++++------ include/haproxy/event_hdl.h | 6 +++--- src/event_hdl.c | 18 ++++++++++++++++-- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/doc/internals/api/event_hdl.txt b/doc/internals/api/event_hdl.txt index 4414bc495..102aea4f6 100644 --- a/doc/internals/api/event_hdl.txt +++ b/doc/internals/api/event_hdl.txt @@ -144,8 +144,8 @@ Registering a handler comes into multiple flavors: sync mode here is that 'unsafe' data provided by the data structure may not be used. task: - handler is a user defined task that uses an event queue - to consume pending events. + handler is a user defined task(let) that uses an event + queue to consume pending events. This mode is interesting when you need to perform advanced operations or you need to handle the event in an already existing task context. @@ -395,9 +395,9 @@ by event_hdl facility to push you events according to your subscription: ``` -Then, you need to declare a tasklet (or reuse existing tasklet) +Then, you need to declare a task(let) (or reuse existing task(let)) -It is your responsibility to make sure that the tasklet still exists +It is your responsibility to make sure that the task(let) still exists (is not freed) when calling the subscribe function (and that the task remains valid as long as the subscription is). @@ -485,9 +485,9 @@ to perform the subscription: ``` Note: it is not recommended to perform multiple subscriptions - that share the same event queue or same tasklet (or both) + that share the same event queue or same task(let) (or both) - That is, having more than one subscription waking a tasklet + That is, having more than one subscription waking a task(let) and/or feeding the same event queue. No check is performed on this when registering, so the API diff --git a/include/haproxy/event_hdl.h b/include/haproxy/event_hdl.h index 097f881fe..2770d1d74 100644 --- a/include/haproxy/event_hdl.h +++ b/include/haproxy/event_hdl.h @@ -176,7 +176,7 @@ uint64_t event_hdl_id(const char *scope, const char *name); * to perform subscription lookup by id * : pointer to event_hdl_async_event queue where the pending * events will be pushed. Cannot be NULL. - * : pointer to tasklet responsible for consuming the events. + * : pointer to task(let) responsible for consuming the events. * Cannot be NULL. * <_private>: pointer to private data that will be handled to * <_private_free>: pointer to 'event_hdl_private_free' prototyped function @@ -187,7 +187,7 @@ uint64_t event_hdl_id(const char *scope, const char *name); (struct event_hdl){ .id = _id, \ .dorigin = _EVENT_HDL_CALLING_PLACE, \ .async = EVENT_HDL_ASYNC_MODE_ADVANCED, \ - .async_task = task, \ + .async_task = (struct tasklet *)task, \ .async_equeue = equeue, \ .private = _private, \ .private_free = _private_free } @@ -201,7 +201,7 @@ uint64_t event_hdl_id(const char *scope, const char *name); * * : pointer to event_hdl_async_event queue where the pending * events will be pushed. Cannot be NULL. - * : pointer to tasklet responsible for consuming the events + * : pointer to task(let) responsible for consuming the events * Cannot be NULL. * <_private>: pointer to private data that will be handled to * <_private_free>: pointer to 'event_hdl_private_free' prototyped function diff --git a/src/event_hdl.c b/src/event_hdl.c index a9fa93a89..37a4adcc2 100644 --- a/src/event_hdl.c +++ b/src/event_hdl.c @@ -196,6 +196,20 @@ void event_hdl_async_free_event(struct event_hdl_async_event *e) pool_free(pool_head_sub_event, e); } +/* wakeup the task depending on its type: + * normal async mode internally uses tasklets but advanced async mode + * allows both tasks and tasklets. + * While tasks and tasklets may be easily casted, we need to use the proper + * API to wake them up (the waiting queues are exclusive). + */ +static void event_hdl_task_wakeup(struct tasklet *task) +{ + if (TASK_IS_TASKLET(task)) + tasklet_wakeup(task); + else + task_wakeup((struct task *)task, TASK_WOKEN_OTHER); /* TODO: switch to TASK_WOKEN_EVENT? */ +} + /* task handler used for normal async subscription mode * if you use advanced async subscription mode, you can use this * as an example to implement your own task wrapper @@ -306,7 +320,7 @@ static inline void _event_hdl_unsubscribe(struct event_hdl_sub *del_sub) lock = MT_LIST_APPEND_LOCKED(del_sub->hdl.async_equeue, &del_sub->async_end->mt_list); /* wake up the task */ - tasklet_wakeup(del_sub->hdl.async_task); + event_hdl_task_wakeup(del_sub->hdl.async_task); /* unlock END EVENT (we're done, the task is now free to consume it) */ MT_LIST_UNLOCK_ELT(&del_sub->async_end->mt_list, lock); @@ -774,7 +788,7 @@ static int _event_hdl_publish(event_hdl_sub_list *sub_list, struct event_hdl_sub MT_LIST_APPEND(cur_sub->hdl.async_equeue, &new_event->mt_list); /* wake up the task */ - tasklet_wakeup(cur_sub->hdl.async_task); + event_hdl_task_wakeup(cur_sub->hdl.async_task); } /* end async mode */ } /* end hdl should be notified */ } /* end mt_list */