Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751773Ab1FNOXV (ORCPT ); Tue, 14 Jun 2011 10:23:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54392 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751114Ab1FNOXT (ORCPT ); Tue, 14 Jun 2011 10:23:19 -0400 Date: Tue, 14 Jun 2011 16:20:23 +0200 From: Oleg Nesterov To: Srikar Dronamraju Cc: Peter Zijlstra , Ingo Molnar , Steven Rostedt , Linux-mm , Arnaldo Carvalho de Melo , Linus Torvalds , Hugh Dickins , Christoph Hellwig , Andi Kleen , Thomas Gleixner , Jonathan Corbet , Andrew Morton , Jim Keniston , Roland McGrath , Ananth N Mavinakayanahalli , LKML Subject: Re: [PATCH v4 3.0-rc2-tip 2/22] 2: uprobes: Breakground page replacement. Message-ID: <20110614142023.GA5139@redhat.com> References: <20110607125804.28590.92092.sendpatchset@localhost6.localdomain6> <20110607125835.28590.25476.sendpatchset@localhost6.localdomain6> <20110613170020.GA27137@redhat.com> <20110614123530.GC4952@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110614123530.GC4952@linux.vnet.ibm.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2173 Lines: 63 On 06/14, Srikar Dronamraju wrote: > > > > +static int write_opcode(struct task_struct *tsk, struct uprobe * uprobe, > > > + unsigned long vaddr, uprobe_opcode_t opcode) > > > +{ > > > + struct page *old_page, *new_page; > > > + void *vaddr_old, *vaddr_new; > > > + struct vm_area_struct *vma; > > > + unsigned long addr; > > > + int ret; > > > + > > > + /* Read the page with vaddr into memory */ > > > + ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 1, 1, &old_page, &vma); > > > > Sorry if this was already discussed... But why we are using FOLL_WRITE here? > > We are not going to write into this page, and this provokes the unnecessary > > cow, no? > > Yes, We are not going to write to the page returned by get_user_pages > but a copy of that page. Yes I see. But the page returned by get_user_pages(write => 1) is already a cow'ed copy (this mapping should be read-only). > The idea was if we cow the page then we dont > need to cow it at the replace_page time Yes, replace_page() shouldn't cow. > and since get_user_pages knows > the right way to cow the page, we dont have to write another routine to > cow the page. Confused. write_opcode() allocs another page and does memcpy. This is correct, but I don't understand the first cow. > I am still not clear on your concern. Probably I missed something... but could you please explain why we can't - ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 1, 1, &old_page, &vma); + ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 0, 0, &old_page, &vma); ? > > Also. This is called under down_read(mmap_sem), can't we race with > > access_process_vm() modifying the same memory? > > Yes, we could be racing with access_process_vm on the same memory. > > Do we have any other option other than making write_opcode/read_opcode > being called under down_write(mmap_sem)? I dunno. Probably we can simply ignore this issue, there are other ways to modify this memory. Oleg. -- 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/