On Tue, 2014-04-22 at 16:47 +0200, Oleg Nesterov wrote: > Finally we can move arch_uprobe->fixups/rip_rela_target_address > into the new "def" struct and place this struct in the union, they > are only used by default_xol_ops paths. > > The patch also renames rip_rela_target_address to riprel_target just > to make this name shorter. > > Signed-off-by: Oleg Nesterov <[email protected]>
I see a couple of nits in this patch (see below), but the others look good. Patches 1-5 of this set: Reviewed-by: Jim Keniston <[email protected]> > --- > arch/x86/include/asm/uprobes.h | 12 ++++++---- > arch/x86/kernel/uprobes.c | 41 +++++++++++++++++++-------------------- > 2 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h > index 93bee7b..72caff7 100644 > --- a/arch/x86/include/asm/uprobes.h > +++ b/arch/x86/include/asm/uprobes.h > @@ -41,18 +41,20 @@ struct arch_uprobe { > u8 ixol[MAX_UINSN_BYTES]; > }; > > - u16 fixups; > const struct uprobe_xol_ops *ops; > > union { > -#ifdef CONFIG_X86_64 > - unsigned long rip_rela_target_address; > -#endif > struct { > s32 offs; > u8 ilen; > u8 opc1; > - } branch; > + } branch; > + struct { > +#ifdef CONFIG_X86_64 > + long riprel_target; > +#endif > + u16 fixups; > + } def; "def" is kind of ambiguous. How about "dfault" or some such? > }; > }; > > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c > index e6314a2..69b2d61 100644 > --- a/arch/x86/kernel/uprobes.c > +++ b/arch/x86/kernel/uprobes.c ... > @@ -636,12 +635,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe > *auprobe, struct mm_struct *mm, > > /* > * Figure out which fixups arch_uprobe_post_xol() will need to perform, > - * and annotate arch_uprobe->fixups accordingly. To start with, ->fixups > - * is either zero or it reflects rip-related fixups. > + * and annotate def->fixups accordingly. To start with, ->fixups is > + * either zero or it reflects rip-related fixups. That sentence stopped being true a couple of patch sets ago. handle_riprel_insn() is called later in this function now. > */ > switch (OPCODE1(&insn)) { > case 0x9d: /* popf */ > - auprobe->fixups |= UPROBE_FIX_SETF; > + auprobe->def.fixups |= UPROBE_FIX_SETF; > break; > case 0xc3: /* ret or lret -- ip is correct */ > case 0xcb: > @@ -669,9 +668,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, > struct mm_struct *mm, > } > > if (fix_ip) > - auprobe->fixups |= UPROBE_FIX_IP; > + auprobe->def.fixups |= UPROBE_FIX_IP; > if (fix_call) > - auprobe->fixups |= UPROBE_FIX_CALL; > + auprobe->def.fixups |= UPROBE_FIX_CALL; > > auprobe->ops = &default_xol_ops; > return 0; Jim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
