sdt: Tear down probes in kernel modules during kldunload

[jhb]

Summary

Previously only providers in kernel modules were removed leaving
dangling pointers to tracepoints, etc. in unloaded kernel modules.

PR: 281825
Reported by: Sony Arpita Das <[e-mail]>
Sponsored by: Chelsio Communications

[markj]

Could you please add a Fixes: ddf0ed09bd8f8 tag to the commit?

sys/cddl/dev/sdt/sdt.c

487

I'd expect this case (i.e., tp2 != NULL) to always be true. I'm ok with keeping the code as-is, but then I'd expect the sdt_tracepoint_valid() test above to be unnecessary.

493

Similarly, this shouldn't happen.

541

Urgh. I should have just fixed this long ago, but it's hard now without breaking compatibility - SDT probe macros shouldn't let one fill in the "module" (or "function") components of the probe. Different modules shouldn't be referencing the same probe, as the kld name is supposed to be part of the probe name. If we followed that convention (and the SDT macros didn't permit deviation from it), this complexity wouldn't be needed.

[jhb]

sys/cddl/dev/sdt/sdt.c

487

I've coded this defensively on the assumption that we might have previously tried to unload the module, and it failed part way during kldunload (e.g. a driver detach routine failed). Then a future attempt to unload the module a second time would do bad things. Or maybe kldload itself failed because a MOD_LOAD handler failed so things aren't fully setup when this is called to tear down.

541

Hmmm, in the case of the bug I opened though it is a probe in the kernel that modules reference because the probes are in static inline functions in <sys/mbuf.h> where tracepoints get inlined into the various modules.

[jhb]

sys/cddl/dev/sdt/sdt.c

487

In particular, I would have just used STAILQ_REMOVE() if we want to assume tp is in the list.

514

Simlarly, this would just use TAILQ_REMOVE() if we weren't being defensive.