Introduce EVENTHANDLER_LIST_* and some users.

[mjoras]

Summary

This introduces a facility to EVENTHANDLER(9) for explicitly defining a reference to an event handler list. This is useful since previously all invokers of events had to do a locked traversal of the global list of event handler lists in order to find the appropriate event handler list. By keeping a pointer to the appropriate list an invoker can avoid this traversal completely. The pointer is initialized with SYSINIT(9) during the eventhandler stage. Users registering interest in events do not need to know if the event is backed by such a list, since the list is added to the global list of lists as normal. As with lists that are not pre-defined it is safe to register for the events before the list has been created.

This converts the process_* and thread_* events to using the new facility, as these are events whose traversals end up showing up significantly in ports build workflows. It may be advantageous to convert other events to using the new facility as well.

Test Plan

buildworlds, bulk port builds by mjg with -j80, kernel module that
registers/deregisters and invokes handlers concurrently

[op]

Could you please update the EVENTHANDLER(9) man page too?

[bdrewery]

Looks good, thanks for doing this. Yes do please update EVENTHANDLER.9

[markj]

Seems ok to me. Do you plan to update this further in light of Ian's similar patch?

+1 to the man page update.

sys/kern/subr_eventhandler.c

143

Style: missing newline after definition, and initialization should be done in a separate statement.

[mjoras]

In D12814#267063, @markj wrote:

Seems ok to me. Do you plan to update this further in light of Ian's similar patch?

+1 to the man page update.

Yeah I've discussed it with Ian. I'll be appropriating the parts of his revision that allow these handlers to be defined from modules (as opposed to my version which only work in the core kernel by design). I'll update the revision shortly with that and the manpage.

[mjoras]

• use a pointer as the global reference to the pre-defined list, allowing for these lists to be created on-register (e.g. from a module) before they are defined. This was the approach suggested and taken by ian in his revision.
• refactor API usage to reflect this change. LISTS are now defined and declared separately from the eventhandlers. The corresponding invoke is EVENTHANDLER_DIRECT_INVOKE since I think EVENTHANDLER_LIST_INVOKE is too confusing (whereas DIRECT can reasonably imply direct disaptch-ish semantics).
• Now you must use EVENTHANDLER_LIST_DEFINE to create the reference to the list. EVENTHANDLER_LIST_DECLARE is only needed to declare the list pointer extern, so for callers of EVENTHANDLER_DIRECT_INVOKE that are not in the same compilation unit as the DEFINE.
• small change to make the eventhandler list name stored at the end of the struct, since the storage is always allocated at the end of the struct anyway.
• remove EHL_INITTED since it is no longer a relevant state (the lists are initialized either by the SYSINIT stage for eventhandler or when the first handler is registered).
• update EVENTHANDLER(9) to reflect the API addition, with the recommendation to use the EVENTHANDLER_LIST/DIRECT macros.

[mjoras]

• style nit

[markj]

share/man/man9/EVENTHANDLER.9

89

"even relatively" sounds a bit weird. Relative to what? I'd just remove that qualifier.

I'm also confused by "additionally." Either we're using the dynamic eventhandler interface, or we aren't, right?

93

New sentences should start on new lines.

177

New sentence, new line.

sys/kern/subr_eventhandler.c

80

Missing spaces around "|".

111

Extra newline.

sys/sys/eventhandler.h

123

What's the point of this flag? The test is racy anyway, so it seems like you could directly check TAILQ_EMPTY(&el->el_entries).

193

I think it's ok to defer the corresponding updates for VIMAGE until they're actually needed, but do you foresee any problems with adding "fast" eventhandlers for VIMAGE?

[mjoras]

share/man/man9/EVENTHANDLER.9

89

It does sound weird. I meant relative to other events but you're right, it's not really a useful addition.

There is no non-dynamic interface in this newer revision. I said "additionally" to try to convey that the EVENTHANDLER_LIST macros are completely extra, they don't actually replace any of the other macros (unlike my previous version). The EVENTHANDLER_LIST_DEFINE is just required if you choose to use EVENTHANDLER_DIRECT_INVOKE. There's nothing stopping someone from using EVENTHANDLER_INVOKE with a pre-defined event handler list; it would work but it would also be a mistake.

sys/sys/eventhandler.h

123

Not really any point. I originally made it as a flag since the original implementation still had a use for EHL_INITTED, so it seemed reasonable to have another flag. I can go ahead and just axe the flags entirely.

[mjoras]

• manpage style and extraneous use of "relative"
• remove the flags field as it is not needed.

[mjoras]

• style fixups.

[mjoras]

sys/sys/eventhandler.h

193

Just looked at how it is implemented. There's still no users of the code interestingly so I'm curious why bz added it.

It seems like it would be compatible without any work. It essentially just overrides the callback function for the underlying handler with one that iterates over all the vnets. It leverages eventhandler_register_internal to register that handler, which is compatible with pre-defined event handler lists. Since vnets don't need to define their own lists or anything it seems like it should "just work", but there's no users to test :/.

[markj]

share/man/man9/EVENTHANDLER.9

89

I see.

[mjoras]

• in order to be MFCable we can't change the eventhandler_list layout.