Page MenuHomeFreeBSD

eventhandler: add support for epoch-backed lists
AbandonedPublic

Authored by melifaro on Aug 11 2022, 7:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 4, 7:59 AM
Unknown Object (File)
Thu, Sep 12, 10:17 AM
Unknown Object (File)
Sep 8 2024, 5:08 AM
Unknown Object (File)
Sep 7 2024, 8:17 PM
Unknown Object (File)
Sep 5 2024, 8:56 AM
Unknown Object (File)
Aug 24 2024, 1:46 PM
Unknown Object (File)
Aug 10 2024, 3:31 PM
Unknown Object (File)
Jul 29 2024, 7:20 PM
Subscribers

Details

Summary

Currently invoking eventhandler(9) event via fastest method, EVENTHANDLER_DIRECT_INVOKE(), involves locking/unlocking event handler list mutex on each call. This may be a bit problematic for events running with high frequency.

The diff below adds another variation of eventhandler(9), leveraging epoch for list traversal.
New KPI semantics: EVENTHANDLER_DEFINE_EPOCH(name, epoch_pointer), EVENTHANDLER_REGISTER_EPOCH(name, epoch, func, arg, prority), EVENTHANDLER_INVOKE_EPOCH(name).

Implementation details:

Item list in the eventhandler_list had to be converted from TAILQ to the LIST, as there is no TAILQ support in ck. In fact, the only place where TAILQ was really used was the worst case when registering new listener. There, the code first traversed the entire list to compare the relative priorities and used tail insertion only if all other priorities were better. Converting the rest invocations to the CK_LIST was straight-forwarded.

Internally eventhandler_list stores pointer to epoch_t. The reason is that event handler lists are allocated at SI_SUB_EVENTHANDLER time, which is much earlier than SI_SUB_EPOCH (where epochs are allocated). Thus, the code stores pointer to the variable that will hold the epoch pointer instead.

Test Plan

System boots & all tests passes.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46866
Build 43755: arc lint + arc unit

Event Timeline

melifaro edited the test plan for this revision. (Show Details)
melifaro added reviewers: mjoras, glebius, bz, thompsa.

This is slower than it needs to be for no good reason. There are already numerous other eventhandler uses which don't execute within network epoch (or any other) and entering one is quite expensive (an atomic op with a barrier). Instead I had a wip patch here to use rms locks: https://reviews.freebsd.org/D27252

I can finish it up.

An important note is that the current API supports callback with unbounded sleep. This can't be done while within epoch. So which handler were you trying to switch to this to begin with and in what conditions is it executing?

In D36143#820635, @mjg wrote:

This is slower than it needs to be for no good reason. There are already numerous other eventhandler uses which don't execute within network epoch (or any other) and entering one is quite expensive (an atomic op with a barrier). Instead I had a wip patch here to use rms locks: https://reviews.freebsd.org/D27252

Thanks for sharing! That's a good one by itself & also good discussion/context inside.

I can finish it up.

An important note is that the current API supports callback with unbounded sleep. This can't be done while within epoch. So which handler were you trying to switch to this to begin with and in what conditions is it executing?

What I have as a use case is the interaction between two potentially unloadable modules (netlink & rtsock) which needs to exchange routing events between each other. I came out with a code that was pretty close to what event handler does, which led me to the idea of using eventhandler instead.
The problem here is that these events can be triggered with pretty high rate (100k/sec in some cases). As all of this happens within the network epoch, which led me to the this diff.

I agree that there is a generic problem and it should be solved in a generic way so all customers could benefit. I'd love to see something better than we have now (especially given I'll be adding more event handlers / their users as a part of netlink introduction). rms looks like a good approach to me - smr requires critical section & we don't have pcpu-based refcounts atm. I'll stick to the custom callbacks model in the netlink/rtsock for now but would love to converge to the event handler api if/when D27252 lands.