Re: [PATCH] uprobes: Document xol_area and arch_uprobe->insn/ixol

From: Ingo Molnar
Date: Tue Nov 19 2013 - 14:24:29 EST



* Oleg Nesterov <oleg@xxxxxxxxxx> wrote:

> On 11/11, Ingo Molnar wrote:
> >
> > * Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >
> > > On 11/11, Ingo Molnar wrote:
> > > >
> > > > You guys are changing code that reads like gobbledygook to people
> > > > reading it for the first time.
> > >
> > > Not that I am trying to defense uprobes, but this is equally true for
> > > any piece of kernel code, at least to me ;)
> >
> > I'm really not suggesting to do overly much - only for some minimal blurb
> > like the scheduler has in most places:
>
> OK. Let me try to make a first step to improve this a little bit...
>
> How about the patch below? Srikar?
>
>
> Subject: [PATCH] uprobes: Document xol_area and arch_uprobe->insn/ixol
>
> Document xol_area and arch_uprobe.
>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
> kernel/events/uprobes.c | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 51a7f53..b886a5e 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -73,6 +73,17 @@ struct uprobe {
> struct inode *inode; /* Also hold a ref to inode */
> loff_t offset;
> unsigned long flags;
> +
> + /*
> + * The generic code assumes that it has two members of unknown type
> + * owned by the arch-specific code:
> + *
> + * insn - copy_insn() saves the original instruction here for
> + * arch_uprobe_analyze_insn().
> + *
> + * ixol - potentially modified instruction to execute out of
> + * line, copied to xol_area by xol_get_insn_slot().
> + */
> struct arch_uprobe arch;
> };
>
> @@ -86,6 +97,10 @@ struct return_instance {
> };
>
> /*
> + * Execute out of line area: anonymous executable mapping installed
> + * by the probed task to execute the copy of the original instruction
> + * mangled by set_swbp().
> + *
> * On a breakpoint hit, thread contests for a slot. It frees the
> * slot after singlestep. Currently a fixed number of slots are
> * allocated.

Looks perfect to me!

Thanks,

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