Hi Masami,
> - /* Step 5: Kick optimizer again if needed. But if there is a flush
> requested, */
> - if (completion_done(&optimizer_completion))
> - complete(&optimizer_completion);
> + /* Step 2: Dispatch waiting room generation if allowed */
> + optprobe_dispatch_generation();
>
> - if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list))
> - kick_kprobe_optimizer(); /*normal kick*/
> + /* Step 3: Check completion if flush was requested */
> + if (!optprobe_optimizer_busy()) {
> + if (optprobe_flush_requested) {
> + optprobe_flush_requested = false;
> + complete_all(&optimizer_completion);
> + }
Removing the completion_done() check also fixes a hang in the current
code: completion_done() is only true once the completion has been
completed, so complete() is never called while a flusher waits. For
example, "echo 0 > /proc/sys/debug/kprobes-optimization" with an
optimized kprobe never returns. I posted a fix for that separately [1].
[...]
> - while (!list_empty(&optimizing_list) ||
> !list_empty(&unoptimizing_list)) {
> + while (optprobe_optimizer_busy()) {
> init_completion(&optimizer_completion);
This part is still racy, though. The flusher drops kprobe_mutex while
it sleeps, and the sysctl handler and the debugfs "enabled" file do not
otherwise serialize against each other. A second flusher can then run
init_completion() on the wait queue the first one is sleeping on: the
first waiter is dropped from the queue, and complete_all() wakes only
the second one.
With this patch the window is wider, since kprobe_mutex is released
while the Tasks RCU grace period runs and optprobe_optimizer_busy()
stays true until the generation is finalized.
The fix in [1] replaces the completion with a counter of optimizer
passes, bumped at the end of each pass and signalled with
wake_up_var_locked(), both under kprobe_mutex; the flusher samples the
count and waits with wait_var_event_mutex() until it changes.
Something similar should fit here, with the counter bumped at the end
of each kprobe_optimizer() call and the flusher re-checking
optprobe_optimizer_busy() under kprobe_mutex as it does now.
[1] https://lore.kernel.org/all/[email protected]/
Thanks,
Andrea
