Nicholas Piggin wrote:
Naveen N. Rao's on June 19, 2019 7:53 pm:
Nicholas Piggin wrote:
Michael Ellerman's on June 19, 2019 3:14 pm:
I'm also not convinced the ordering between the two patches is
guaranteed by the ISA, given that there's possibly no isync on the other
CPU.
Will they go through a context synchronizing event?
synchronize_rcu_tasks() should ensure a thread is scheduled away, but
I'm not actually sure it guarantees CSI if it's kernel->kernel. Could
do a smp_call_function to do the isync on each CPU to be sure.
Good point. Per
Documentation/RCU/Design/Requirements/Requirements.html#Tasks RCU:
"The solution, in the form of Tasks RCU, is to have implicit read-side
critical sections that are delimited by voluntary context switches, that
is, calls to schedule(), cond_resched(), and synchronize_rcu_tasks(). In
addition, transitions to and from userspace execution also delimit
tasks-RCU read-side critical sections."
I suppose transitions to/from userspace, as well as calls to schedule()
result in context synchronizing instruction being executed. But, if some
tasks call cond_resched() and synchronize_rcu_tasks(), we probably won't
have a CSI executed.
Also:
"In CONFIG_PREEMPT=n kernels, trampolines cannot be preempted, so these
APIs map to call_rcu(), synchronize_rcu(), and rcu_barrier(),
respectively."
In this scenario as well, I think we won't have a CSI executed in case
of cond_resched().
Should we enhance patch_instruction() to handle that?
Well, not sure. Do we have many post-boot callers of it? Should
they take care of their own synchronization requirements?
Kprobes and ftrace are the two users (along with anything else that may
use jump labels).
Looking at this from the CMODX perspective: the main example quoted of
an erratic behavior is when any variant of the patched instruction
causes an exception.
With ftrace, I think we are ok since we only ever patch a 'nop' or a
'bl' (and the 'mflr' now), none of which should cause an exception. As
such, the existing patch_instruction() should suffice.
However, with kprobes, we patch a 'trap' (or a branch in case of
optprobes) on most instructions. I wonder if we should be issuing an
'isync' on all cpus in this case. Or, even if that is sufficient or
necessary.
Thanks,
Naveen