Re: [PATCH V3 3/4] arch/powerpc: Implement Optprobes

[Naveen N. Rao]

On 2017/02/07 10:05AM, Masami Hiramatsu wrote:
> On Sat, 4 Feb 2017 01:09:49 +0530
> "Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > Hi Michael,
> > Thanks for the review! I'll defer to Anju on most of the aspects, but...
> > 
> > On 2017/02/01 09:53PM, Michael Ellerman wrote:
> > > Anju T Sudhakar <anju@xxxxxxxxxxxxxxxxxx> writes:
> > > 
> > > > +static void optimized_callback(struct optimized_kprobe *op,
> > > > +			       struct pt_regs *regs)
> > > > +{
> > > > +	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> > > > +	unsigned long flags;
> > > > +
> > > > +	/* This is possible if op is under delayed unoptimizing */
> > > > +	if (kprobe_disabled(&op->kp))
> > > > +		return;
> > > > +
> > > > +	local_irq_save(flags);
> > > 
> > > What is that protecting against? Because on powerpc it doesn't actually
> > > disable interrupts, it just masks some of them, the perf interrupt for
> > > example can still run.
> > 
> > That's an excellent catch, as always! :)
> > 
> > This is meant to prevent us from missing kprobe hits while processing 
> > interrupts that arrive when this optprobe is being handled. And you are 
> > totally right -- we would miss kprobe hits during PMI handling with the 
> > current approach. We need a hard_irq_disable() there.
> 
> One note: it depends on the arch implementation of kprobes, since this
> is only for "emulating" the int3 behavior on x86 for compatibility.
> On x86, int3 is disabling interrupt automatically, so all the kprobes
> user handlers will be run under irq-disabled. This means that user may
> write their code to run as such condition. They even can not know
> that is optimized or not at programming timing, because the kprobe
> will be optimized after a while from enabled it.
> 
> So the important point is that you have to keep it compatible of
> unoptimized kprobes.

Thanks for the pointers, Masami.
Yes, with unoptimized kprobes on powerpc, we run with interrupts hard 
disabled as well. So, we need to hard disable for optprobes too.

Regards,
Naveen