On Fri, 18 Oct 2013 16:27:24 +0200
Petr Mladek <pmla...@suse.cz> wrote:
> void ftrace_replace_code(int enable)
> {
> struct ftrace_rec_iter *iter;
> struct dyn_ftrace *rec;
> - const char *report = "adding breakpoints";
> - int count = 0;
> - int ret;
> + unsigned long *addr = NULL;
> + void *opcode = NULL, *op = NULL;
> + const unsigned char *new;
> + size_t count = 0;
>
> for_ftrace_rec_iter(iter) {
> rec = ftrace_rec_iter_record(iter);
> -
> - ret = add_breakpoints(rec, enable);
> - if (ret)
> - goto remove_breakpoints;
> - count++;
> - }
> -
> - run_sync();
> -
> - report = "updating code";
> -
> - for_ftrace_rec_iter(iter) {
> - rec = ftrace_rec_iter_record(iter);
> -
> - ret = add_update(rec, enable);
> - if (ret)
> - goto remove_breakpoints;
> + new = ftrace_new_code(rec, enable);
> +
> + /* check next record if there is no new code for this one */
> + if (!new)
> + continue;
> +
> + /* allocate buffers only when there will be a change */
> + if (unlikely(!addr)) {
> + if (ftrace_allocate_replace_buffers(&addr, &opcode))
> + goto out;
> + op = opcode;
> + count = 0;
> + }
> +
> + /* fill buffers */
> + addr[count++] = rec->ip;
> + memcpy(op, new, MCOUNT_INSN_SIZE);
> + op += MCOUNT_INSN_SIZE;
> +
> + /* write buffers if they are full */
> + if (count == FTRACE_MAX_RECS_PATCH) {
> + text_poke_bp((void **)addr, opcode,
> + MCOUNT_INSN_SIZE, count, NULL);
> + op = opcode;
> + count = 0;
> + }
> }
>
> - run_sync();
> -
> - report = "removing breakpoints";
> -
> - for_ftrace_rec_iter(iter) {
> - rec = ftrace_rec_iter_record(iter);
> + if (count)
> + text_poke_bp((void **)addr, opcode,
> + MCOUNT_INSN_SIZE, count, NULL);
>
> - ret = finish_update(rec, enable);
> - if (ret)
> - goto remove_breakpoints;
> - }
> -
> - run_sync();
> -
> - return;
> -
> - remove_breakpoints:
> - ftrace_bug(ret, rec ? rec->ip : 0);
> - printk(KERN_WARNING "Failed on %s (%d):\n", report, count);
> - for_ftrace_rec_iter(iter) {
> - rec = ftrace_rec_iter_record(iter);
> - remove_breakpoint(rec);
Also you must keep the ftrace_bug() functionality. I notice there's no
error check for text_poke_bp(). Not only do we need to check the error
status, but if an error does happen, we must know the following:
1) What record it failed on
2) Did in fail on the read, compare, or write?
If it failed on the read: -EFAULT is returned
If it failed on the compare: -EINVAL is returned
If it failed on the write: -EPERM is returned
Look at "ftrace_bug()", and then you can also do a google search on
"ftrace faulted" or "ftrace failed to modify", and see that this is an
extremely useful debugging tool, and not something I will allow to be
removed.
-- Steve
> - }
> +out:
> + kfree(addr);
> + kfree(opcode);
> }
>
--
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/
