On Fri, 27 Apr 2018 08:56:36 +0200
Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> * Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> > @@ -712,7 +712,7 @@ static void reuse_unused_kprobe(struct kprobe *ap)
> > op = container_of(ap, struct optimized_kprobe, kp);
> > if (unlikely(list_empty(&op->list)))
> > printk(KERN_WARNING "Warning: found a stray unused "
> > - "aggrprobe@%p\n", ap->addr);
> > + "aggrprobe@%pS\n", ap->addr);
>
> Is this a kernel bug? If yes then please convert this to a WARN_ON_ONCE() that
> doesn't print any kernel addresses.
OK, I will do.
>
> > /* Enable the probe again */
> > ap->flags &= ~KPROBE_FLAG_DISABLED;
> > /* Optimize it again (remove from op->list) */
> > @@ -985,7 +985,8 @@ static int arm_kprobe_ftrace(struct kprobe *p)
> > ret = ftrace_set_filter_ip(&kprobe_ftrace_ops,
> > (unsigned long)p->addr, 0, 0);
> > if (ret) {
> > - pr_debug("Failed to arm kprobe-ftrace at %p (%d)\n", p->addr, ret);
> > + pr_debug("Failed to arm kprobe-ftrace at %pS (%d)\n",
> > + p->addr, ret);
> > return ret;
>
> Can this happen during normal use? If yes then just remove the warning message.
Yes, it can, for example probing on live-patched function.
>
> > }
> >
> > @@ -1025,7 +1026,8 @@ static int disarm_kprobe_ftrace(struct kprobe *p)
> >
> > ret = ftrace_set_filter_ip(&kprobe_ftrace_ops,
> > (unsigned long)p->addr, 1, 0);
> > - WARN(ret < 0, "Failed to disarm kprobe-ftrace at %p (%d)\n", p->addr, ret);
> > + WARN(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
> > + p->addr, ret);
>
> As this is a signal of a kernel bug, just convert this to a regular WARN_ONCE()
> that doesn't print any kernel addresses.
Would you mean just replace WARN with WARN_ONCE as below?
WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
p->addr, ret);
Anyway, without printing kprobe->addr, it may be meaningless for this case.
>
> > +/* Caller must NOT call this in usual path. This is only for critical case */
> > void dump_kprobe(struct kprobe *kp)
> > {
> > - printk(KERN_WARNING "Dumping kprobe:\n");
> > - printk(KERN_WARNING "Name: %s\nAddress: %p\nOffset: %x\n",
> > - kp->symbol_name, kp->addr, kp->offset);
> > + pr_err("Dumping kprobe:\n");
> > + pr_err("Name: %s\nOffset: %x\nAddress: %pS\n",
> > + kp->symbol_name, kp->offset, kp->addr);
>
> No, this function should just go away and be replaced by a WARN() in
> reenter_kprobe().
Would you consider to use pr_err() here? If so, I'll move this
dump as you suggested.
Thank you,
--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>