| Message ID | 20140411152207.GA28188@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 11, 2014 at 05:22:07PM +0200, Oleg Nesterov wrote: > On 04/11, Oleg Nesterov wrote: > > > > Can't we do _something_ > > like below? > > If not, I'd propose the patch below. > > I can be easily wrong, but it seems that arch/arm can reimplement > arch_uprobe_flush_xol_icache() and do flush_ptrace_access()-like > code. It needs kaddr, but this is not a problem. > > Btw. From arch/arm/include/asm/cacheflush.h > > #define flush_icache_user_range(vma,page,addr,len) \ > flush_dcache_page(page) > > but it has no users? I wonder whether you've read this yet: http://lkml.iu.edu//hypermail/linux/kernel/1404.1/00725.html where I proposed removing flush_icache_user_range() since it's not used on a great many architectures. > And I am just curious, why arm's copy_to_user_page() disables premption > before memcpy? flush_ptrace_access() needs to run on the CPU which ended up with the dirty cache line(s) to cope with those which do not have hardware broadcasting of cache maintanence operations. This is why the hacks that you're doing are just that - they're hacks and are all broken in some way. So, let me re-quote what I asked in a previous thread: | Given that we've already solved that problem, wouldn't it be a good idea | if the tracing code would stop trying to reinvent broken solutions to | problems we have already solved? I fail to see what your problem is with keeping the vma around, and using that infrastructure. If it needs optimisation for uprobes, then let's optimise it. Let's not go inventing a whole new interface solving the same problem.
On Fri, Apr 11, 2014 at 05:22:07PM +0200, Oleg Nesterov wrote: > And I am just curious, why arm's copy_to_user_page() disables premption > before memcpy? Without looking, I suspect its because the VIVT caches, they need to get shot down on every context switch.
On Fri, Apr 11, 2014 at 05:32:49PM +0200, Peter Zijlstra wrote: > On Fri, Apr 11, 2014 at 05:22:07PM +0200, Oleg Nesterov wrote: > > And I am just curious, why arm's copy_to_user_page() disables premption > > before memcpy? > > Without looking, I suspect its because the VIVT caches, they need to get > shot down on every context switch. So... let's think about that for a moment... if we have a preemption event, then that's a context switch, which means... No, this is obviously not the reason, because such an event on a fully VIVT system would result in the caches being flushed, meaning that we wouldn't need to do anything if we could be predicably preempted at that point.
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Fri, 11 Apr 2014 16:30:41 +0100 > | Given that we've already solved that problem, wouldn't it be a good idea > | if the tracing code would stop trying to reinvent broken solutions to > | problems we have already solved? > > I fail to see what your problem is with keeping the vma around, and > using that infrastructure. If it needs optimisation for uprobes, then > let's optimise it. Let's not go inventing a whole new interface > solving the same problem. +1
On Fri, Apr 11, 2014 at 05:00:29PM +0100, Russell King - ARM Linux wrote: > On Fri, Apr 11, 2014 at 05:32:49PM +0200, Peter Zijlstra wrote: > > On Fri, Apr 11, 2014 at 05:22:07PM +0200, Oleg Nesterov wrote: > > > And I am just curious, why arm's copy_to_user_page() disables premption > > > before memcpy? > > > > Without looking, I suspect its because the VIVT caches, they need to get > > shot down on every context switch. > > So... let's think about that for a moment... if we have a preemption event, > then that's a context switch, which means... > > No, this is obviously not the reason, because such an event on a fully > VIVT system would result in the caches being flushed, meaning that we > wouldn't need to do anything if we could be predicably preempted at that > point. Yeah; I've since realized I was completely wrong about that. Thanks for explaining though.
--- x/kernel/events/uprobes.c +++ x/kernel/events/uprobes.c @@ -1274,6 +1274,17 @@ static unsigned long xol_take_insn_slot( return slot_addr; } +void __weak arch_uprobe_flush_xol_icache(struct page *page, + unsigned long vaddr, int len) +{ + /* + * We need copy_to_user_page/flush_icache_user_range but this + * needs vma. If this doesn't work on your arch, reimplement. + */ + flush_dcache_page(area->page); + +} + /* * xol_get_insn_slot - allocate a slot for xol. * Returns the allocated slot address or 0. @@ -1294,11 +1305,8 @@ static unsigned long xol_get_insn_slot(s /* Initialize the slot */ copy_to_page(area->page, xol_vaddr, &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); - /* - * We probably need flush_icache_user_range() but it needs vma. - * This should work on supported architectures too. - */ - flush_dcache_page(area->page); + arch_uprobe_flush_xol_icache(area->page, xol_vaddr, + sizeof(uprobe->arch.ixol)); return xol_vaddr; }