From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32470 invoked by alias); 6 Feb 2009 02:06:09 -0000 Received: (qmail 31450 invoked by uid 22791); 6 Feb 2009 02:06:07 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx3.mail.elte.hu (HELO mx3.mail.elte.hu) (157.181.1.138) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 06 Feb 2009 02:06:01 +0000 Received: from elvis.elte.hu ([157.181.1.14]) by mx3.mail.elte.hu with esmtp (Exim) id 1LVG69-000587-3N from ; Fri, 06 Feb 2009 03:05:47 +0100 Received: by elvis.elte.hu (Postfix, from userid 1004) id DCA1A3E21B0; Fri, 6 Feb 2009 03:05:35 +0100 (CET) Date: Fri, 06 Feb 2009 03:42:00 -0000 From: Ingo Molnar To: Mathieu Desnoyers Cc: Masami Hiramatsu , Andrew Morton , Linus Torvalds , Greg KH , Nick Piggin , LKML , Ananth N Mavinakayanahalli , Jim Keniston , systemtap-ml , "Frank Ch. Eigler" Subject: Re: [BUGFIX][PATCH -rc/-mm] prevent kprobes from catching spurious page faults Message-ID: <20090206020537.GA32618@elte.hu> References: <497FE895.1080708@redhat.com> <20090128154824.GA6025@Krystal> <49808EEF.1020700@redhat.com> <20090128171331.GA9006@Krystal> <49809CCE.40409@redhat.com> <20090128181053.GC9908@Krystal> <498B6457.20302@redhat.com> <20090205235727.GA16040@elte.hu> <20090206011320.GA7161@Krystal> <20090206020429.GB13937@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090206020429.GB13937@elte.hu> User-Agent: Mutt/1.5.18 (2008-05-17) Received-SPF: neutral (mx3: 157.181.1.14 is neither permitted nor denied by domain of elte.hu) client-ip=157.181.1.14; envelope-from=mingo@elte.hu; helo=elvis.elte.hu; X-ELTE-VirusStatus: clean X-ELTE-SpamScore: -1.5 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-1.5 required=5.9 tests=BAYES_00 autolearn=no SpamAssassin version=3.2.3 -1.5 BAYES_00 BODY: Bayesian spam probability is 0 to 1% [score: 0.0000] X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2009-q1/txt/msg00386.txt.bz2 * Ingo Molnar wrote: > > * Mathieu Desnoyers wrote: > > > * Ingo Molnar (mingo@elte.hu) wrote: > > > > > > * Masami Hiramatsu wrote: > > > > > > > - if (notify_page_fault(regs)) > > > > - return; > > > > if (unlikely(kmmio_fault(regs, address))) > > > > return; > > > > > > > > @@ -634,6 +632,9 @@ void __kprobes do_page_fault(struct pt_r > > > > if (spurious_fault(address, error_code)) > > > > return; > > > > > > > > + /* kprobes don't want to hook the spurious faults. */ > > > > + if (notify_page_fault(regs)) > > > > + return; > > > > /* > > > > * Don't take the mm semaphore here. If we fixup a prefetch > > > > * fault we could otherwise deadlock. > > > > @@ -641,6 +642,9 @@ void __kprobes do_page_fault(struct pt_r > > > > goto bad_area_nosemaphore; > > > > } > > > > > > > > + /* kprobes don't want to hook the spurious faults. */ > > > > + if (notify_page_fault(regs)) > > > > + return; > > > > > > I dont know - this spreads that callback to two places now. Any > > > reason why kprobes cannot call spurious_fault(), if there's a > > > probe active? > > > > > > Also, moving that would remove the planned cleanup of merging these > > > two into one call: > > > > > > if (notify_page_fault(regs)) > > > return; > > > if (unlikely(kmmio_fault(regs, address))) > > > return; > > > > > > We should reduce the probing cross section, not increase it, > > > especially in such a critical codepath as the pagefault handler. > > > > > > Btw., why cannot kprobes install a dynamic probe to the fault > > > handler itself? That way the default path would have no such > > > callbacks and checks at all. > > > > > > > Or we could simply merge my 2 LTTng page fault handler tracepoints per > > architecture and be done with it ? > > > > I'd need to clean up the patchset a little bit to fold a few patches, > > but that would be straightforward enough. > > yes, that would be an option too - it depends on the details of how it looks > like and what kind of complexity it hides. Linus just merged the fix so the urgency of the matter has become lower :) Ingo