Re: [PATCH -v5 12/17] x86/kprobes: Fix ordering

[Mathieu Desnoyers]

----- On Nov 14, 2019, at 10:28 AM, Peter Zijlstra peterz@xxxxxxxxxxxxx wrote:

> On Thu, Nov 14, 2019 at 10:22:24AM -0500, Mathieu Desnoyers wrote:
> 
>> >> > So what we do, after enabling the regular kprobe, is call
>> >> > synchronize_rcu_tasks() to wait for each task to have passed through
>> >> > schedule(). That guarantees no task is preempted inside the kprobe
>> >> > shadow (when it triggers it ensures it resumes execution at an
>> >> > instruction boundary further than 5 bytes away).
>> >> 
>> >> Indeed, given that synchronize_rcu_tasks() awaits for voluntary context
>> >> switches (or user-space execution), it guarantees that no task was preempted
>> >> within the kprobe shadow.
>> >> 
>> >> Considering that synchronize_rcu_tasks() is meant only for code rewriting,
>> >> I wonder if it would make sense to include the core serializing guarantees
>> >> within this RCU API ?
>> > 
>> > As in have synchronize_rcu_tasks() do the IPI-sync love before doing
>> > the current wait-for-voluntary-context-switch work?
>> 
>> This is what I have in mind, yes, based on the assumption that the only
>> intended use-case for synchronize_rcu_tasks() is code patching.
> 
> I don't think that is needed. As per the patch under discussion, we
> unconditionally need that IPI-sync (even for !optimized) but we only
> need the synchonize_rcu_tasks() thing for optimized kprobes.
> 
> Also, they really do two different things. Lets not tie them together.

I'm fine with this approach, I just thought it would be good to consider
the alternative.

Thanks!

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com