]> sourceware.org Git - systemtap.git/commit
runtime: remove stap_utrace_detach_ops() from task_finder2
authorSultan Alsawaf <sultan@openresty.com>
Wed, 27 Apr 2022 01:36:05 +0000 (18:36 -0700)
committerSultan Alsawaf <sultan@openresty.com>
Tue, 24 May 2022 19:52:06 +0000 (12:52 -0700)
commita329b05dd05460360dcee3f29d18cab284016527
treebb9bbef1932a519cf2d8316dbeab7c642e96db0a
parentc0c9a31895d86a4b73497fb3930ef8b004a6b5db
runtime: remove stap_utrace_detach_ops() from task_finder2

This function isn't actually needed because it's guaranteed that all the
utrace engines will be detached by the time stap_utrace_detach_ops() runs,
since utrace_exit() always runs before stap_utrace_detach_ops(). This is
evidenced by the utrace derived probe group being ordered above the
task_finder derived probe group in all_session_groups(): the utrace group
runs stap_utrace_detach_ops() on cleanup, and the task_finder group runs
utrace_exit() on cleanup, and since DOONE(task_finder) comes *after*
DOONE(utrace), the task_finder group's cleanup will always come *before*
the utrace group's cleanup.

The motivation for removing this is twofold: stap_utrace_detach_ops()
incurs cubic runtime complexity (it is called once for each utrace probe,
iterates across every process in the system, and then iterates across every
thread for each process), and it has two serious bugs that'd surface if it
did ever actually find a utrace engine to detach (which will never happen
since all the utrace engines are guaranteed to be detached beforehand, as
explained above).

The two bugs are related to the following call chains:
  stap_stop_task_finder
    utrace_exit
      stp_task_work_exit <-- Dangerous to add task workers after this...
  stap_utrace_detach_ops
    rcu_read_lock <-- RCU read lock acquired...
    stap_utrace_detach(tsk = do_each_thread())
      utrace_control(UTRACE_DETACH)
        utrace_do_stop
          stp_task_notify_resume
            stp_task_work_add <-- BUG: we won't wait for this worker to finish!
      utrace_barrier(tsk != current)
        schedule_timeout_interruptible <-- BUG: scheduling under RCU read lock!
    rcu_read_unlock <-- RCU read lock released...

Since stap_stop_task_finder() always comes before stap_utrace_detach_ops(),
that means stp_task_work_exit() also happens beforehand, which is bad
because stap_utrace_detach_ops() may add a task worker. The added task
worker can thus run after the stap module is unloaded.

The other bug involves sleeping under an RCU read lock, which is expressly
forbidden.

Neither of these bugs can ever actually occur though because there will
never be any utrace engines to detach by the time stap_utrace_detach_ops()
runs. As such, remove stap_utrace_detach_ops() since it is not only buggy,
but also a CPU hog that slows down module unload. The old task_finder.c
used by ancient kernels still has its own copy of stap_utrace_detach_ops()
that's needed, so only skip emitting the stap_utrace_detach_ops() call when
task_finder2.c is used.
runtime/linux/task_finder2.c
tapset-utrace.cxx
This page took 0.027086 seconds and 5 git commands to generate.