Messages in this thread | | | Date | Mon, 14 Jun 2010 13:54:23 -0400 | From | Christoph Hellwig <> | Subject | Re: [PATCH v5 7/14] x86 support for Uprobes |
| |
On Mon, Jun 14, 2010 at 01:59:13PM +0530, Srikar Dronamraju wrote: > @@ -850,7 +850,19 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags) > > if (thread_info_flags & _TIF_UPROBE) { > clear_thread_flag(TIF_UPROBE); > +#ifdef CONFIG_X86_32 > + /* > + * On x86_32, do_notify_resume() gets called with > + * interrupts disabled. Hence enable interrupts if they > + * are still disabled. > + */ > + native_irq_enable(); > +#endif > uprobe_notify_resume(regs); > + > +#ifdef CONFIG_X86_32 > + native_irq_disable(); > +#endif
I'm no x86 port guru, but this looks rather worriesome to me. Why does do_notify_resume have different calling conventions on 32 vs 64-bit? And if there is a good reason that 32-bit has them disabled, why is enabling them in the middle of do_notify_resume okay?
> +void arch_uprobe_disable_sstep(struct pt_regs *regs) > +{ > + /* Disable single-stepping by clearing what we set */ > + clear_thread_flag(TIF_SINGLESTEP); > + clear_thread_flag(TIF_FORCED_TF); > + regs->flags &= ~X86_EFLAGS_TF; > +}
This seems to have one layer of indentation too much.
| |