From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Harvey Harrison <harvey.harrison@gmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@elte.hu>,
Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86: Remove 'e' from kprope structure members
Date: Wed, 12 Dec 2007 15:00:20 -0800 [thread overview]
Message-ID: <47606804.1010709@goop.org> (raw)
In-Reply-To: <1197487642.21291.1.camel@brick>
Harvey Harrison wrote:
> Some kprobe structure members had a superfluous e in their
> name.
>
> eflags -> flags
> esp -> sp
>
eflags and esp are the actual machine register names (at least in
32-bit), and therefore more distinctive than just "flags".
If this is in preparation for a unification then OK, but I disagree if
not (and technically 64-bit should be using rsp/rflags).
J
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> ---
> Ingo, this applies on top of my kprobes unification patch.
>
> arch/x86/kernel/kprobes_32.c | 34 +++++++++++++++++-----------------
> arch/x86/kernel/kprobes_64.c | 34 +++++++++++++++++-----------------
> include/asm-x86/kprobes.h | 10 +++++-----
> 3 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes_32.c b/arch/x86/kernel/kprobes_32.c
> index 6585ec5..44bc600 100644
> --- a/arch/x86/kernel/kprobes_32.c
> +++ b/arch/x86/kernel/kprobes_32.c
> @@ -195,26 +195,26 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> kcb->prev_kprobe.kp = kprobe_running();
> kcb->prev_kprobe.status = kcb->kprobe_status;
> - kcb->prev_kprobe.old_eflags = kcb->kprobe_old_eflags;
> - kcb->prev_kprobe.saved_eflags = kcb->kprobe_saved_eflags;
> + kcb->prev_kprobe.old_flags = kcb->kprobe_old_flags;
> + kcb->prev_kprobe.saved_flags = kcb->kprobe_saved_flags;
> }
>
> static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> __get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
> kcb->kprobe_status = kcb->prev_kprobe.status;
> - kcb->kprobe_old_eflags = kcb->prev_kprobe.old_eflags;
> - kcb->kprobe_saved_eflags = kcb->prev_kprobe.saved_eflags;
> + kcb->kprobe_old_flags = kcb->prev_kprobe.old_flags;
> + kcb->kprobe_saved_flags = kcb->prev_kprobe.saved_flags;
> }
>
> static void __kprobes set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
> struct kprobe_ctlblk *kcb)
> {
> __get_cpu_var(current_kprobe) = p;
> - kcb->kprobe_saved_eflags = kcb->kprobe_old_eflags
> + kcb->kprobe_saved_flags = kcb->kprobe_old_flags
> = (regs->flags & (TF_MASK | IF_MASK));
> if (is_IF_modifier(p->opcode))
> - kcb->kprobe_saved_eflags &= ~IF_MASK;
> + kcb->kprobe_saved_flags &= ~IF_MASK;
> }
>
> static __always_inline void clear_btf(void)
> @@ -280,7 +280,7 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> if (kcb->kprobe_status == KPROBE_HIT_SS &&
> *p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> regs->flags &= ~TF_MASK;
> - regs->flags |= kcb->kprobe_saved_eflags;
> + regs->flags |= kcb->kprobe_saved_flags;
> goto no_kprobe;
> }
> /* We have reentered the kprobe_handler(), since
> @@ -501,7 +501,7 @@ static void __kprobes resume_execution(struct kprobe *p,
> switch (p->ainsn.insn[0]) {
> case 0x9c: /* pushfl */
> *tos &= ~(TF_MASK | IF_MASK);
> - *tos |= kcb->kprobe_old_eflags;
> + *tos |= kcb->kprobe_old_flags;
> break;
> case 0xc2: /* iret/ret/lret */
> case 0xc3:
> @@ -578,7 +578,7 @@ static int __kprobes post_kprobe_handler(struct pt_regs *regs)
> }
>
> resume_execution(cur, regs, kcb);
> - regs->flags |= kcb->kprobe_saved_eflags;
> + regs->flags |= kcb->kprobe_saved_flags;
> trace_hardirqs_fixup_flags(regs->flags);
>
> /*Restore back the original saved kprobes variables and continue. */
> @@ -617,7 +617,7 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> * normal page fault.
> */
> regs->ip = (unsigned long)cur->addr;
> - regs->flags |= kcb->kprobe_old_eflags;
> + regs->flags |= kcb->kprobe_old_flags;
> if (kcb->kprobe_status == KPROBE_REENTER)
> restore_previous_kprobe(kcb);
> else
> @@ -703,8 +703,8 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>
> kcb->jprobe_saved_regs = *regs;
> - kcb->jprobe_saved_esp = ®s->sp;
> - addr = (unsigned long)(kcb->jprobe_saved_esp);
> + kcb->jprobe_saved_sp = ®s->sp;
> + addr = (unsigned long)(kcb->jprobe_saved_sp);
>
> /*
> * TBD: As Linus pointed out, gcc assumes that the callee
> @@ -730,23 +730,23 @@ void __kprobes jprobe_return(void)
> " .globl jprobe_return_end \n"
> " jprobe_return_end: \n"
> " nop \n"::"b"
> - (kcb->jprobe_saved_esp):"memory");
> + (kcb->jprobe_saved_sp):"memory");
> }
>
> int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
> {
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> u8 *addr = (u8 *) (regs->ip - 1);
> - unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_esp);
> + unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_sp);
> struct jprobe *jp = container_of(p, struct jprobe, kp);
>
> if ((addr > (u8 *) jprobe_return) && (addr < (u8 *) jprobe_return_end)) {
> - if (®s->sp != kcb->jprobe_saved_esp) {
> + if (®s->sp != kcb->jprobe_saved_sp) {
> struct pt_regs *saved_regs =
> - container_of(kcb->jprobe_saved_esp,
> + container_of(kcb->jprobe_saved_sp,
> struct pt_regs, sp);
> printk("current sp %p does not match saved sp %p\n",
> - ®s->sp, kcb->jprobe_saved_esp);
> + ®s->sp, kcb->jprobe_saved_sp);
> printk("Saved registers for jprobe %p\n", jp);
> show_registers(saved_regs);
> printk("Current registers\n");
> diff --git a/arch/x86/kernel/kprobes_64.c b/arch/x86/kernel/kprobes_64.c
> index a0ec7c2..3d1f1fa 100644
> --- a/arch/x86/kernel/kprobes_64.c
> +++ b/arch/x86/kernel/kprobes_64.c
> @@ -234,26 +234,26 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> kcb->prev_kprobe.kp = kprobe_running();
> kcb->prev_kprobe.status = kcb->kprobe_status;
> - kcb->prev_kprobe.old_eflags = kcb->kprobe_old_eflags;
> - kcb->prev_kprobe.saved_eflags = kcb->kprobe_saved_eflags;
> + kcb->prev_kprobe.old_flags = kcb->kprobe_old_flags;
> + kcb->prev_kprobe.saved_flags = kcb->kprobe_saved_flags;
> }
>
> static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> __get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
> kcb->kprobe_status = kcb->prev_kprobe.status;
> - kcb->kprobe_old_eflags = kcb->prev_kprobe.old_eflags;
> - kcb->kprobe_saved_eflags = kcb->prev_kprobe.saved_eflags;
> + kcb->kprobe_old_flags = kcb->prev_kprobe.old_flags;
> + kcb->kprobe_saved_flags = kcb->prev_kprobe.saved_flags;
> }
>
> static void __kprobes set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
> struct kprobe_ctlblk *kcb)
> {
> __get_cpu_var(current_kprobe) = p;
> - kcb->kprobe_saved_eflags = kcb->kprobe_old_eflags
> + kcb->kprobe_saved_flags = kcb->kprobe_old_flags
> = (regs->flags & (TF_MASK | IF_MASK));
> if (is_IF_modifier(p->ainsn.insn))
> - kcb->kprobe_saved_eflags &= ~IF_MASK;
> + kcb->kprobe_saved_flags &= ~IF_MASK;
> }
>
> static __always_inline void clear_btf(void)
> @@ -312,7 +312,7 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> if (kcb->kprobe_status == KPROBE_HIT_SS &&
> *p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> regs->flags &= ~TF_MASK;
> - regs->flags |= kcb->kprobe_saved_eflags;
> + regs->flags |= kcb->kprobe_saved_flags;
> goto no_kprobe;
> } else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
> /* TODO: Provide re-entrancy from
> @@ -510,7 +510,7 @@ static void __kprobes resume_execution(struct kprobe *p,
> switch (*insn) {
> case 0x9c: /* pushfl */
> *tos &= ~(TF_MASK | IF_MASK);
> - *tos |= kcb->kprobe_old_eflags;
> + *tos |= kcb->kprobe_old_flags;
> break;
> case 0xc3: /* ret/lret */
> case 0xcb:
> @@ -565,7 +565,7 @@ static int __kprobes post_kprobe_handler(struct pt_regs *regs)
> }
>
> resume_execution(cur, regs, kcb);
> - regs->flags |= kcb->kprobe_saved_eflags;
> + regs->flags |= kcb->kprobe_saved_flags;
> trace_hardirqs_fixup_flags(regs->flags);
>
> /* Restore the original saved kprobes variables and continue. */
> @@ -605,7 +605,7 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> * normal page fault.
> */
> regs->ip = (unsigned long)cur->addr;
> - regs->flags |= kcb->kprobe_old_eflags;
> + regs->flags |= kcb->kprobe_old_flags;
> if (kcb->kprobe_status == KPROBE_REENTER)
> restore_previous_kprobe(kcb);
> else
> @@ -694,8 +694,8 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>
> kcb->jprobe_saved_regs = *regs;
> - kcb->jprobe_saved_esp = (long *) regs->sp;
> - addr = (unsigned long)(kcb->jprobe_saved_esp);
> + kcb->jprobe_saved_sp = (long *) regs->sp;
> + addr = (unsigned long)(kcb->jprobe_saved_sp);
> /*
> * As Linus pointed out, gcc assumes that the callee
> * owns the argument space and could overwrite it, e.g.
> @@ -720,23 +720,23 @@ void __kprobes jprobe_return(void)
> " .globl jprobe_return_end \n"
> " jprobe_return_end: \n"
> " nop \n"::"b"
> - (kcb->jprobe_saved_esp):"memory");
> + (kcb->jprobe_saved_sp):"memory");
> }
>
> int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
> {
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> u8 *addr = (u8 *) (regs->ip - 1);
> - unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_esp);
> + unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_sp);
> struct jprobe *jp = container_of(p, struct jprobe, kp);
>
> if ((addr > (u8 *) jprobe_return) && (addr < (u8 *) jprobe_return_end)) {
> - if ((long *)regs->sp != kcb->jprobe_saved_esp) {
> + if ((long *)regs->sp != kcb->jprobe_saved_sp) {
> struct pt_regs *saved_regs =
> - container_of(kcb->jprobe_saved_esp,
> + container_of(kcb->jprobe_saved_sp,
> struct pt_regs, sp);
> printk("current sp %p does not match saved sp %p\n",
> - (long *)regs->sp, kcb->jprobe_saved_esp);
> + (long *)regs->sp, kcb->jprobe_saved_sp);
> printk("Saved registers for jprobe %p\n", jp);
> show_registers(saved_regs);
> printk("Current registers\n");
> diff --git a/include/asm-x86/kprobes.h b/include/asm-x86/kprobes.h
> index 074ac7d..87b9d1b 100644
> --- a/include/asm-x86/kprobes.h
> +++ b/include/asm-x86/kprobes.h
> @@ -73,16 +73,16 @@ struct arch_specific_insn {
> struct prev_kprobe {
> struct kprobe *kp;
> unsigned long status;
> - unsigned long old_eflags;
> - unsigned long saved_eflags;
> + unsigned long old_flags;
> + unsigned long saved_flags;
> };
>
> /* per-cpu kprobe control block */
> struct kprobe_ctlblk {
> unsigned long kprobe_status;
> - unsigned long kprobe_old_eflags;
> - unsigned long kprobe_saved_eflags;
> - long *jprobe_saved_esp;
> + unsigned long kprobe_old_flags;
> + unsigned long kprobe_saved_flags;
> + long *jprobe_saved_sp;
> struct pt_regs jprobe_saved_regs;
> kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
> struct prev_kprobe prev_kprobe;
>
next prev parent reply other threads:[~2007-12-12 23:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1197421087.8761.27.camel@brick>
[not found] ` <47602776.10408@zytor.com>
2007-12-12 19:27 ` [PATCH] x86: Remove 'e' from kprope structure members Harvey Harrison
2007-12-12 23:00 ` Jeremy Fitzhardinge [this message]
2007-12-12 23:08 ` Harvey Harrison
2007-12-12 23:08 ` H. Peter Anvin
2007-12-12 23:20 ` Jeremy Fitzhardinge
2007-12-12 23:24 ` H. Peter Anvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47606804.1010709@goop.org \
--to=jeremy@goop.org \
--cc=harvey.harrison@gmail.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.