* Re: [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() [not found] <tip-b18018126f422f5b706fd750373425e10e84b486@kernel.org> @ 2009-02-20 23:09 ` Masami Hiramatsu 2009-02-22 9:31 ` Ingo Molnar 0 siblings, 1 reply; 4+ messages in thread From: Masami Hiramatsu @ 2009-02-20 23:09 UTC (permalink / raw) To: hpa, mingo, torvalds, akpm, tglx, mhiramat, mingo, linux-kernel Cc: linux-tip-commits Ingo Molnar wrote: > Author: Ingo Molnar <mingo@elte.hu> > AuthorDate: Fri, 20 Feb 2009 22:42:57 +0100 > Commit: Ingo Molnar <mingo@elte.hu> > CommitDate: Sat, 21 Feb 2009 00:09:42 +0100 > > x86, mm, kprobes: fault.c, simplify notify_page_fault() > > Impact: cleanup > > Remove an #ifdef from notify_page_fault(). The function still > compiles to nothing in the !CONFIG_KPROBES case. > > Introduce kprobes_built_in() and kprobe_fault_handler() helpers > to allow this - they returns 0 if !CONFIG_KPROBES. > > No code changed: > > text data bss dec hex filename > 4618 32 24 4674 1242 fault.o.before > 4618 32 24 4674 1242 fault.o.after It seems good for me. Thank you for cleanup! Acked-by: Masami Hiramatsu <mhiramat@redhat.com> > > Cc: Masami Hiramatsu <mhiramat@redhat.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > --- > arch/x86/mm/fault.c | 6 +----- > include/linux/kprobes.h | 22 +++++++++++++++++++--- > 2 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index fe99af4..379beae 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -68,11 +68,10 @@ static inline int kmmio_fault(struct pt_regs *regs, unsigned long addr) > > static inline int notify_page_fault(struct pt_regs *regs) > { > -#ifdef CONFIG_KPROBES > int ret = 0; > > /* kprobe_running() needs smp_processor_id() */ > - if (!user_mode_vm(regs)) { > + if (kprobes_built_in() && !user_mode_vm(regs)) { > preempt_disable(); > if (kprobe_running() && kprobe_fault_handler(regs, 14)) > ret = 1; > @@ -80,9 +79,6 @@ static inline int notify_page_fault(struct pt_regs *regs) > } > > return ret; > -#else > - return 0; > -#endif > } > > /* > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 32851ee..2ec6cc1 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -182,6 +182,14 @@ struct kprobe_blackpoint { > DECLARE_PER_CPU(struct kprobe *, current_kprobe); > DECLARE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); > > +/* > + * For #ifdef avoidance: > + */ > +static inline int kprobes_built_in(void) > +{ > + return 1; > +} > + > #ifdef CONFIG_KRETPROBES > extern void arch_prepare_kretprobe(struct kretprobe_instance *ri, > struct pt_regs *regs); > @@ -271,8 +279,16 @@ void unregister_kretprobes(struct kretprobe **rps, int num); > void kprobe_flush_task(struct task_struct *tk); > void recycle_rp_inst(struct kretprobe_instance *ri, struct hlist_head *head); > > -#else /* CONFIG_KPROBES */ > +#else /* !CONFIG_KPROBES: */ > > +static inline int kprobes_built_in(void) > +{ > + return 0; > +} > +static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr) > +{ > + return 0; > +} > static inline struct kprobe *get_kprobe(void *addr) > { > return NULL; > @@ -329,5 +345,5 @@ static inline void unregister_kretprobes(struct kretprobe **rps, int num) > static inline void kprobe_flush_task(struct task_struct *tk) > { > } > -#endif /* CONFIG_KPROBES */ > -#endif /* _LINUX_KPROBES_H */ > +#endif /* CONFIG_KPROBES */ > +#endif /* _LINUX_KPROBES_H */ -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: mhiramat@redhat.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() 2009-02-20 23:09 ` [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() Masami Hiramatsu @ 2009-02-22 9:31 ` Ingo Molnar 2009-02-23 16:21 ` Masami Hiramatsu 2009-02-23 18:46 ` Ananth N Mavinakayanahalli 0 siblings, 2 replies; 4+ messages in thread From: Ingo Molnar @ 2009-02-22 9:31 UTC (permalink / raw) To: Masami Hiramatsu, Andrew Morton Cc: hpa, mingo, torvalds, tglx, linux-kernel, linux-tip-commits * Masami Hiramatsu <mhiramat@redhat.com> wrote: > Ingo Molnar wrote: > > Author: Ingo Molnar <mingo@elte.hu> > > AuthorDate: Fri, 20 Feb 2009 22:42:57 +0100 > > Commit: Ingo Molnar <mingo@elte.hu> > > CommitDate: Sat, 21 Feb 2009 00:09:42 +0100 > > > > x86, mm, kprobes: fault.c, simplify notify_page_fault() > > > > Impact: cleanup > > > > Remove an #ifdef from notify_page_fault(). The function still > > compiles to nothing in the !CONFIG_KPROBES case. > > > > Introduce kprobes_built_in() and kprobe_fault_handler() helpers > > to allow this - they returns 0 if !CONFIG_KPROBES. > > > > No code changed: > > > > text data bss dec hex filename > > 4618 32 24 4674 1242 fault.o.before > > 4618 32 24 4674 1242 fault.o.after > > It seems good for me. Thank you for cleanup! > > Acked-by: Masami Hiramatsu <mhiramat@redhat.com> another very small thing, while we are discussing kprobes: I always found that the __kprobes annotation is very confusingly euphemistic: what those annotations really mean is not 'kprobes', but 'no kprobes'. So how about renaming __kprobes to __nokprobes, similar to how we have the notrace attribute? We have about 350 __kprobes annotations in the kernel, so renaming it now would not be practical - but any objections against me sending Linus a rename patch somewhere late in the next merge window that just does this rename? [ likewise, i'll rename notrace to __notrace to make it visually less intrusive to the return value type. There's a lot less such annotations in the kernel. ] Ingo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() 2009-02-22 9:31 ` Ingo Molnar @ 2009-02-23 16:21 ` Masami Hiramatsu 2009-02-23 18:46 ` Ananth N Mavinakayanahalli 1 sibling, 0 replies; 4+ messages in thread From: Masami Hiramatsu @ 2009-02-23 16:21 UTC (permalink / raw) To: Ingo Molnar Cc: Andrew Morton, hpa, mingo, torvalds, tglx, linux-kernel, linux-tip-commits, systemtap-ml, Ananth N Mavinakayanahalli, Jim Keniston Hi Ingo, Ingo Molnar wrote: > another very small thing, while we are discussing kprobes: > > I always found that the __kprobes annotation is very confusingly > euphemistic: what those annotations really mean is not > 'kprobes', but 'no kprobes'. As far as I know, __kprobes originally means 'this function will be called from kprobes'. However, now many functions are tagged as __kprobes, it might be confusingly. > So how about renaming __kprobes to __nokprobes, similar to how > we have the notrace attribute? Would you mean that will include changing section name of '.text.kprobes'? That's what I mind. > We have about 350 __kprobes annotations in the kernel, so > renaming it now would not be practical - but any objections > against me sending Linus a rename patch somewhere late in the > next merge window that just does this rename? Just renaming __kprobes to __nokprobes seems good for me. Thank you! > [ likewise, i'll rename notrace to __notrace to make it visually > less intrusive to the return value type. There's a lot less > such annotations in the kernel. ] > > Ingo > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: mhiramat@redhat.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() 2009-02-22 9:31 ` Ingo Molnar 2009-02-23 16:21 ` Masami Hiramatsu @ 2009-02-23 18:46 ` Ananth N Mavinakayanahalli 1 sibling, 0 replies; 4+ messages in thread From: Ananth N Mavinakayanahalli @ 2009-02-23 18:46 UTC (permalink / raw) To: Ingo Molnar Cc: Masami Hiramatsu, Andrew Morton, hpa, mingo, torvalds, tglx, linux-kernel, linux-tip-commits On Sun, Feb 22, 2009 at 10:31:09AM +0100, Ingo Molnar wrote: > > * Masami Hiramatsu <mhiramat@redhat.com> wrote: > > > Ingo Molnar wrote: > > > Author: Ingo Molnar <mingo@elte.hu> > > > AuthorDate: Fri, 20 Feb 2009 22:42:57 +0100 > > > Commit: Ingo Molnar <mingo@elte.hu> > > > CommitDate: Sat, 21 Feb 2009 00:09:42 +0100 > > > > > > x86, mm, kprobes: fault.c, simplify notify_page_fault() > > > > > > Impact: cleanup > > > > > > Remove an #ifdef from notify_page_fault(). The function still > > > compiles to nothing in the !CONFIG_KPROBES case. > > > > > > Introduce kprobes_built_in() and kprobe_fault_handler() helpers > > > to allow this - they returns 0 if !CONFIG_KPROBES. > > > > > > No code changed: > > > > > > text data bss dec hex filename > > > 4618 32 24 4674 1242 fault.o.before > > > 4618 32 24 4674 1242 fault.o.after > > > > It seems good for me. Thank you for cleanup! > > > > Acked-by: Masami Hiramatsu <mhiramat@redhat.com> > > another very small thing, while we are discussing kprobes: > > I always found that the __kprobes annotation is very confusingly > euphemistic: what those annotations really mean is not > 'kprobes', but 'no kprobes'. Right! > So how about renaming __kprobes to __nokprobes, similar to how > we have the notrace attribute? > > We have about 350 __kprobes annotations in the kernel, so > renaming it now would not be practical - but any objections > against me sending Linus a rename patch somewhere late in the > next merge window that just does this rename? No issues with that. Ananth ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-02-23 18:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <tip-b18018126f422f5b706fd750373425e10e84b486@kernel.org>
2009-02-20 23:09 ` [tip:x86/mm] x86, mm, kprobes: fault.c, simplify notify_page_fault() Masami Hiramatsu
2009-02-22 9:31 ` Ingo Molnar
2009-02-23 16:21 ` Masami Hiramatsu
2009-02-23 18:46 ` Ananth N Mavinakayanahalli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox