On Thu, 2013-07-11 at 22:26 +0200, Jiri Kosina wrote:

Nitpick, and Joe Perches mentioned this earlier too. The below should be
in kerneldoc format. That is:

 /**
  * text_poke_bp() -- update instructions on live kernel on SMP


> +/*
> + * text_poke_bp() -- update instructions on live kernel on SMP
> + * @addr:    address to patch
> + * @opcode:  opcode of new instruction
> + * @len:     length to copy
> + * @handler: address to jump to when the temporary breakpoint is hit
> + *
> +
> + * Modify multi-byte instruction by using int3 breakpoint on SMP.

Also, you have a missing asterisk.

See Documentation/kernel-doc-nano-HOWTO.txt for more info.


But other than that, you can add my:

Reviewed-by: Steven Rostedt <rost...@goodmis.org>

-- Steve

> + * In contrary to text_poke_smp(), we completely avoid stop_machine() here,
> + * and achieve the synchronization using int3 breakpoint.
> + *
> + * The way it is done:
> + *   - add a int3 trap to the address that will be patched
> + *   - sync cores
> + *   - update all but the first byte of the patched range
> + *   - sync cores
> + *   - replace the first byte (int3) by the first byte of
> + *     replacing opcode
> + *   - sync cores
> + *
> + * Note: must be called under text_mutex.
> + */
> +void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler)
> +{
> +     unsigned char int3 = 0xcc;
> +
> +     bp_int3_handler = handler;
> +     bp_int3_addr = (u8 *)addr + sizeof(int3);
> +     bp_patching_in_progress = true;
> +     /*
> +      * Corresponding read barrier in int3 notifier for
> +      * making sure the in_progress flags is correctly ordered wrt.
> +      * patching
> +      */
> +     smp_wmb();
> +
> +     text_poke(addr, &int3, sizeof(int3));
> +
> +     on_each_cpu(do_sync_core, NULL, 1);
> +
> +     if (len - sizeof(int3) > 0) {
> +             /* patch all but the first byte */
> +             text_poke((char *)addr + sizeof(int3),
> +                       (const char *) opcode + sizeof(int3),
> +                       len - sizeof(int3));
> +             /*
> +              * According to Intel, this core syncing is very likely
> +              * not necessary and we'd be safe even without it. But
> +              * better safe than sorry (plus there's not only Intel).
> +              */
> +             on_each_cpu(do_sync_core, NULL, 1);
> +     }
> +
> +     /* patch the first byte */
> +     text_poke(addr, opcode, sizeof(int3));
> +
> +     on_each_cpu(do_sync_core, NULL, 1);
> +
> +     bp_patching_in_progress = false;
> +     smp_wmb();
> +
> +     return addr;
> +}
> +
> +/* this one needs to run before anything else handles it as a
> + * regular exception */
> +static struct notifier_block int3_nb = {
> +     .priority = 0x7fffffff,
> +     .notifier_call = int3_notify
> +};
> +
> +static int __init int3_init(void)
> +{
> +     return register_die_notifier(&int3_nb);
> +}
> +
> +arch_initcall(int3_init);
>  /*
>   * Cross-modifying kernel text with stop_machine().
>   * This code originally comes from immediate value.
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index bddf3b2..d6db7bd 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1709,7 +1709,7 @@ EXPORT_SYMBOL_GPL(unregister_kprobes);
>  
>  static struct notifier_block kprobe_exceptions_nb = {
>       .notifier_call = kprobe_exceptions_notify,
> -     .priority = 0x7fffffff /* we need to be notified first */
> +     .priority = 0x7ffffff0 /* High priority, but not first.  */
>  };
>  
>  unsigned long __weak arch_deref_entry_point(void *entry)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to