With emit_module_refresh, we can support probes in modules that weren't loaded at the time stap started. However, if a module is unloaded and then loaded again, we miss the chance to register the probes anew. For example, on a system not already using btrfs.ko: $ stap -e 'probe module("btrfs").function("*_btrfs_fs").call { println(ppfunc()) }' -c 'sudo sh -c "modprobe btrfs; modprobe -r btrfs; modprobe btrfs; modprobe -r btrfs"' init_btrfs_fs exit_btrfs_fs The init and exit are only noticed once, despite being loaded twice.
OK, I've looked into this a bit more. It seems like we do register and unregister when it "makes sense", i.e. - Upon MODULE_STATE_COMING, we update all the addresses, and then register all kprobes. - Upon MODULE_STATE_LIVE, we zero out any init section, and then unregister any init kprobes. - Upon MODULE_STATE_GOING, we zero out all sections, and then unregister all kprobes. However, it looks like kprobes doesn't like it when we unregister things during the MODULE_STATE_LIVE and MODULE_STATE_GOING events. This may be due to the fact that kprobes already checks for these events as well (and is notified before us) and accordingly disarms no longer valid kprobes (see kprobes_module_callback()). For that reason, unregistration for no longer valid module addresses is not necessary. If we instead unregister and re-register during the MODULE_STATE_COMING event, then everything works as expected. Note by the way that this is the approach also done in trace_kprobe.c (see trace_kprobe_module_callback()). Tentative patch to follow.
Created attachment 7647 [details] Patch In this patch, we only call back to systemtap_module_refresh() for the COMING event. We also pass the module name so that only kprobes for that module are refreshed. With the patch: $ stap -e 'probe module("btrfs").function("*_btrfs_fs").call { print ln(ppfunc()) }' -c 'sudo sh -c "modprobe btrfs; modprobe -r btrfs; modprobe btrfs; modprobe -r btrfs"' init_btrfs_fs exit_btrfs_fs init_btrfs_fs exit_btrfs_fs $
Created attachment 7648 [details] Patch Sorry, previous patch was the wrong one. :)
OK, I've decided to *really* look into this and see why kprobes seems to be misbehaving. The answer is that we are the ones actually misbehaving. :) The key is that we are actually being notified *before* the kprobes callback, not after (I believe the comments in transport.c which is based on kernel/trace/trace_kprobe.c, are wrong -- notifiers are called from high to low priority number, not the other way around, see notifier_chain_register()). So what would happen is that (prior to the patch I posted) we would call unregister_kprobe() during the GOING event, which would set the DISABLED flag as part of the process. That DISABLED flag stayed in the struct and is what caused the behaviour in the PR in the first place (i.e. the next register() looked like it didn't work, when really it did register but just remained disabled). On the other hand, with the patch I posted above, we would call unregister_kprobe() only at the next COMING event, at which point it doesn't bother to set the DISABLED flag because kprobes had already flagged it as GONE during the previous GOING event (see the kprobe_disabled() predicate, which looks at both the DISABLED and the GONE flags). This is why the following register() call worked. So really all we needed to do was change the priority of the notifier to be less than that of the kprobes callback and of course make sure that the flags member is cleared (or just zero out the whole struct).
The fix for this is part of the jlebon/onthefly branch (commit 19d62b5), which has now been merged into master (merge commit 2e9f9de).