From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2746 invoked by alias); 22 Dec 2005 05:09:20 -0000 Received: (qmail 2738 invoked by uid 22791); 22 Dec 2005 05:09:19 -0000 X-Spam-Status: No, hits=-0.9 required=5.0 tests=AWL,BAYES_00,DNS_FROM_RFC_POST X-Spam-Check-By: sourceware.org Received: from fmr17.intel.com (HELO orsfmr002.jf.intel.com) (134.134.136.16) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 22 Dec 2005 05:09:17 +0000 Received: from orsfmr100.jf.intel.com (orsfmr100.jf.intel.com [10.7.209.16]) by orsfmr002.jf.intel.com (8.12.10/8.12.10/d: major-outer.mc,v 1.1 2004/09/17 17:50:56 root Exp $) with ESMTP id jBM598bo016000; Thu, 22 Dec 2005 05:09:08 GMT Received: from pdsmsxvs01.pd.intel.com (pdsmsxvs01.pd.intel.com [172.16.12.122]) by orsfmr100.jf.intel.com (8.12.10/8.12.10/d: major-inner.mc,v 1.2 2004/09/17 18:05:01 root Exp $) with SMTP id jBM58TgL023893; Thu, 22 Dec 2005 05:09:05 GMT Received: from pdsmsx331.ccr.corp.intel.com ([172.16.12.58]) by pdsmsxvs01.pd.intel.com (SAVSMTP 3.1.7.47) with SMTP id M2005122213090512684 ; Thu, 22 Dec 2005 13:09:05 +0800 Received: from pdsmsx404.ccr.corp.intel.com ([172.16.12.64]) by pdsmsx331.ccr.corp.intel.com with Microsoft SMTPSVC(6.0.3790.211); Thu, 22 Dec 2005 13:09:05 +0800 X-MimeOLE: Produced By Microsoft Exchange V6.5.7226.0 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="GB2312" Content-Transfer-Encoding: quoted-printable Subject: RE: Review patches of user space kprobe Date: Thu, 22 Dec 2005 05:34:00 -0000 Message-ID: <8126E4F969BA254AB43EA03C59F44E840447BCF6@pdsmsx404> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Review patches of user space kprobe Thread-Index: AcYFUmXZTeBzN902Ro6gR+ou3K1AMwAn0RrgACsLjBAABfJB8A== From: "Zhang, Yanmin" To: Cc: , "Keshavamurthy, Anil S" , "Mao, Bibo" X-OriginalArrivalTime: 22 Dec 2005 05:09:05.0224 (UTC) FILETIME=[D4985C80:01C606B5] X-Scanned-By: MIMEDefang 2.52 on 10.7.209.16 X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2005-q4/txt/msg00539.txt.bz2 One more comment for patch 1/3. register_aggr_kprobe needs to be aware of user space probe. Yanmin >>-----Original Message----- >>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.o= rg] On Behalf Of Zhang, Yanmin >>Sent: 2005=C4=EA12=D4=C222=C8=D5 11:37 >>To: prasanna@in.ibm.com >>Cc: systemtap@sources.redhat.com; Keshavamurthy, Anil S; Mao, Bibo >>Subject: RE: Review patches of user space kprobe >> >>General question: >>1) How to insert an uprobe at anonymous page (VMA)? I think there are 2 c= ases related to the question. >> a) Many applications execute codes produced themselves, such like JIT (J= ust-In-Time) of JVM. >> b) Some executables include TEXTREL section. When they are loaded into m= emory and linked dynamically, the text section might be >>changed, and kernel will do a Copy-On-Write to create a new anonymous pag= e and map the new page to the process address space. So after >>the process starts, we couldn't insert uprobe on its copied pages. >>Should a new interface be added to support it? The parameters could be pr= ocess id and offset in the process address space. Of course, >>it could be an enhancement and implemented later. >> >>3) Can function register_userspace_probe do not call register_kprobe? I t= hink it's not necessary. It's just my feeling. It's up to you >>to make decision. :) >> >>2) Function get_inode_ops should take care of errors and its caller, regi= ster_userspace_probe should check if the return value of >>get_inode_ops is IS_ERR. If so, the error code should be returned instead= of a hard-coded -ENOSYS. >> >>Other more comments against patch 1/3 are inline below. >> >>Yanmin >> >>>>-----Original Message----- >>>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware= .org] On Behalf Of Zhang, Yanmin >>>>Sent: 2005=C4=EA12=D4=C221=C8=D5 16:30 >>>>To: prasanna@in.ibm.com >>>>>>+static struct vm_area_struct *find_get_vma(unsigned long offset, >>>>>>+ struct address_space >>>>*mapping) >>>>>>+{ >>>>>>+ struct vm_area_struct *vma =3D NULL; >>>>>>+ struct prio_tree_iter iter; >>>>>>+ struct prio_tree_root *head =3D &mapping->i_mmap; >>>>>>+ struct mm_struct *mm; >>>>>>+ unsigned long start, end; >>>>>>+ >>>>>>+ spin_lock(&mapping->i_mmap_lock); >>>>>>+ vma_prio_tree_foreach(vma, &iter, head, offset, offset) { >>>>>>+ mm =3D vma->vm_mm; >>>>>>+ spin_lock(&mm->page_table_lock); >>>>>>+ start =3D vma->vm_start - (vma->vm_pgoff << PAGE_SHIFT); >>>>>>+ end =3D vma->vm_end - (vma->vm_pgoff << PAGE_SHIFT); >>>>>>+ spin_unlock(&mm->page_table_lock); >>>>>>+ if ((start + offset) < end) { >>>>>>+ spin_unlock(&mapping->i_mmap_lock); >>>>>>+ return vma; >>It's not safe to return vma without lock. There is a race condition. If v= ma is released by another thread, kernel might be crazy when >>this thread tries to access it later. >> >>If the page is mapped to many vma in different processes, function find_g= et_vma just returns one vma. It's not enough. >> >>I'd like to suggest to do the flush_icache in the vma_prio_tree_foreach l= oop. >> >> >>>>>>+ } >>>>>>+ } >>>>>>+ spin_unlock(&mapping->i_mmap_lock); >>>>>>+ return NULL; >>>>>>+}