Hi Jessica,

Since this is under discussion about ftrace part, I just comment another one.

On Thu,  2 Nov 2017 17:33:33 +0100
Jessica Yu <j...@kernel.org> wrote:

[...]
> @@ -1362,9 +1372,14 @@ static int register_aggr_kprobe(struct kprobe *orig_p, 
> struct kprobe *p)
>  
>       if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
>               ap->flags &= ~KPROBE_FLAG_DISABLED;
> -             if (!kprobes_all_disarmed)
> +             if (!kprobes_all_disarmed) {
>                       /* Arm the breakpoint again. */
> -                     arm_kprobe(ap);
> +                     ret = arm_kprobe(ap);
> +                     if (ret) {
> +                             ap->flags |= KPROBE_FLAG_DISABLED;
> +                             list_del_rcu(&p->list);

You also have to wait rcu here.

> +                     }
> +             }
>       }
>       return ret;
>  }
[...]
> @@ -2428,16 +2452,26 @@ static void arm_all_kprobes(void)
>       /* Arming kprobes doesn't optimize kprobe itself */
>       for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>               head = &kprobe_table[i];
> -             hlist_for_each_entry_rcu(p, head, hlist)
> -                     if (!kprobe_disabled(p))
> -                             arm_kprobe(p);
> +             /* Arm all kprobes on a best-effort basis */
> +             hlist_for_each_entry_rcu(p, head, hlist) {
> +                     if (!kprobe_disabled(p)) {
> +                             err = arm_kprobe(p);
> +                             if (err)  {
> +                                     errors++;
> +                                     ret = err;
> +                             }
> +                     }
> +             }
>       }
>  
> -     printk(KERN_INFO "Kprobes globally enabled\n");
> +     if (errors)
> +             pr_warn("Kprobes globally enabled, but failed to arm %d 
> kprobes\n", errors);

The last "kprobes" should be "probes" because it can include kretprobes :P

Thank you,

-- 
Masami Hiramatsu <mhira...@kernel.org>

Reply via email to