From patchwork Fri Apr 11 14:56:25 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oleg Nesterov X-Patchwork-Id: 3970011 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5FA089F336 for ; Fri, 11 Apr 2014 15:15:40 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7790B20783 for ; Fri, 11 Apr 2014 15:15:39 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5ACA020776 for ; Fri, 11 Apr 2014 15:15:38 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYd8e-0001eN-W6; Fri, 11 Apr 2014 15:13:08 +0000 Received: from merlin.infradead.org ([2001:4978:20e::2]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYd8c-0001cY-Su for linux-arm-kernel@bombadil.infradead.org; Fri, 11 Apr 2014 15:13:06 +0000 Received: from mx1.redhat.com ([209.132.183.28]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYctJ-0005nQ-Oy for linux-arm-kernel@lists.infradead.org; Fri, 11 Apr 2014 14:57:19 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s3BEuHlp007479 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 11 Apr 2014 10:56:17 -0400 Received: from tranklukator.brq.redhat.com (dhcp-1-104.brq.redhat.com [10.34.1.104]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id s3BEuD3b000333; Fri, 11 Apr 2014 10:56:14 -0400 Received: by tranklukator.brq.redhat.com (nbSMTP-1.00) for uid 500 oleg@redhat.com; Fri, 11 Apr 2014 16:56:29 +0200 (CEST) Date: Fri, 11 Apr 2014 16:56:25 +0200 From: Oleg Nesterov To: David Miller , Linus Torvalds , Peter Zijlstra Subject: Re: [RFC PATCH] uprobes: copy to user-space xol page with proper cache flushing Message-ID: <20140411145625.GA27493@redhat.com> References: <20140409184507.GA1058@redhat.com> <5347655B.3080307@linaro.org> <20140411.003636.272212797007496394.davem@davemloft.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140411.003636.272212797007496394.davem@davemloft.net> User-Agent: Mutt/1.5.18 (2008-05-17) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140411_105717_973626_98259048 X-CRM114-Status: GOOD ( 27.34 ) X-Spam-Score: -7.3 (-------) Cc: tixy@linaro.org, linaro-kernel@lists.linaro.org, linux@arm.linux.org.uk, ananth@in.ibm.com, victor.kamensky@linaro.org, taras.kondratiuk@linaro.org, rabin@rab.in, dave.long@linaro.org, Dave.Martin@arm.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP First of all: I do not pretend I really understand the problems with icache/etc coherency and how flush_icache_range() actually works on alpha. Help. For those who were not cc'ed. A probed task has a special "xol" vma, it is used to execute the probed insn out of line. Initial implementation was x86 only, so we simply copied the probed insn into this vma and everything was fine, flush_icache_range() is nop on x86. Then we added flush_dcache_page() for powerpc, // this is just kmap() + memcpy() 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); but this doesn't work on arm. So we need another fix. On 04/11, David Miller wrote: > > From: David Long > Date: Thu, 10 Apr 2014 23:45:31 -0400 > > > Replace memcpy and dcache flush in generic uprobes with a call to > > copy_to_user_page(), which will do a proper flushing of kernel and > > user cache. Also modify the inmplementation of copy_to_user_page > > to assume a NULL vma pointer means the user icache corresponding > > to this right is stale and needs to be flushed. Note that this patch > > does not fix copy_to_user page for the sh, alpha, sparc, or mips > > architectures (which do not currently support uprobes). > > > > Signed-off-by: David A. Long > > You really need to pass the proper VMA down to the call site > rather than pass NULL, that's extremely ugly and totally > unnecesary. I agree that we should not change copy_to_user_page(), but I am not sure the code above should use copy_to_user_page(). Because the code above really differs in my opinion from the only user of copy_to_user_page(), __access_remote_vm(). 1. First of all, we do not know vma. OK, we can down_read(mmap_sem) and do find_vma() of course. This is a bit unfortunate, especially because the architectures we currently support do not need this. But, 2. The problem is, it would be very nice to remove this vma, or at least hide it somehow from find_vma/etc. This is the special mapping we do not want to expose to user-space. In fact I even have the patches which remove this vma, but they do not work with compat tasks unfortunately. 3. Unlike __access_remote_vm() we always use current->mm, and the memory range changed by the code above can only be used by "current" thread. So (perhaps) flush_icache_user_range() can even treat this case as "mm->mm_users) <= 1" and avoid ipi_flush_icache_page (just in case, of course I do not know if this is actually possible). So can't we do something else? Lets forget about arch/arm for the moment, suppose that we want to support uprobes on alpha. Can't we do _something_ like below? And perhap we can do something like this on arm? it can do kmap(page) / page_address(page) itself. Oleg. --- x/arch/alpha/kernel/smp.c +++ x/arch/alpha/kernel/smp.c @@ -751,15 +751,9 @@ ipi_flush_icache_page(void *x) flush_tlb_other(mm); } -void -flush_icache_user_range(struct vm_area_struct *vma, struct page *page, - unsigned long addr, int len) -{ - struct mm_struct *mm = vma->vm_mm; - - if ((vma->vm_flags & VM_EXEC) == 0) - return; +void __flush_icache_page_xxx(struct mm_struct *mm, struct page *page) // addr, len ? +{ preempt_disable(); if (mm == current->active_mm) { @@ -783,3 +777,24 @@ flush_icache_user_range(struct vm_area_s preempt_enable(); } + +void flush_icache_page_xxx(struct mm_struct *mm, struct page *page) +{ + struct mm_struct *mm = current->mm; + + down_read(&mm->mmap_sem); + __flush_icache_page_xxx(mm, page); + up_read(&mm->mmap_sem); +} + +void +flush_icache_user_range(struct vm_area_struct *vma, struct page *page, + unsigned long addr, int len) +{ + struct mm_struct *mm = vma->vm_mm; + + if ((vma->vm_flags & VM_EXEC) == 0) + return; + + __flush_icache_page_xxx(mm, page); +}