On 7/25/23 09:36, Valentin Schneider wrote:
On 25/07/23 06:49, Joel Fernandes wrote:
Interesting series Valentin. Some high-level question/comments on this one:
On Jul 20, 2023, at 12:34 PM, Valentin Schneider vschneid@redhat.com wrote:
text_poke_bp_batch() sends IPIs to all online CPUs to synchronize them vs the newly patched instruction. CPUs that are executing in userspace do not need this synchronization to happen immediately, and this is actually harmful interference for NOHZ_FULL CPUs.
Does the amount of harm not correspond to practical frequency of text_poke? How often does instruction patching really happen? If it is very infrequent then I am not sure if it is that harmful.
Being pushed over a latency threshold *once* is enough to impact the latency evaluation of your given system/application.
It's mainly about shielding the isolated, NOHZ_FULL CPUs from whatever the housekeeping CPUs may be up to (flipping static keys, loading kprobes, using ftrace...) - frequency of the interference isn't such a big part of the reasoning.
Makes sense.
As the synchronization IPIs are sent using a blocking call, returning from text_poke_bp_batch() implies all CPUs will observe the patched instruction(s), and this should be preserved even if the IPI is deferred. In other words, to safely defer this synchronization, any kernel instruction leading to the execution of the deferred instruction sync (ct_work_flush()) must *not* be mutable (patchable) at runtime.
If it is not infrequent, then are you handling the case where userland spends multiple seconds before entering the kernel, and all this while the blocking call waits? Perhaps in such situation you want the real IPI to be sent out instead of the deferred one?
The blocking call only waits for CPUs for which it queued a CSD. Deferred calls do not queue a CSD thus do not impact the waiting at all. See smp_call_function_many_cond().
Ah I see you are using on_each_cpu_cond(). I should have gone through the other patch before making noise.
thanks,
- Joel