This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [BUGFIX][PATCH -rc/-mm] prevent kprobes from catching spurious page faults
- From: Ingo Molnar <mingo at elte dot hu>
- To: Mathieu Desnoyers <mathieu dot desnoyers at polymtl dot ca>
- Cc: Masami Hiramatsu <mhiramat at redhat dot com>, Andrew Morton <akpm at linux-foundation dot org>, Linus Torvalds <torvalds at linux-foundation dot org>, Greg KH <greg at kroah dot com>, Nick Piggin <npiggin at suse dot de>, LKML <linux-kernel at vger dot kernel dot org>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Jim Keniston <jkenisto at us dot ibm dot com>, systemtap-ml <systemtap at sources dot redhat dot com>, "Frank Ch. Eigler" <fche at redhat dot com>
- Date: Fri, 6 Feb 2009 03:05:37 +0100
- Subject: Re: [BUGFIX][PATCH -rc/-mm] prevent kprobes from catching spurious page faults
- 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>
* Ingo Molnar <mingo@elte.hu> wrote:
>
> * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
>
> > * Ingo Molnar (mingo@elte.hu) wrote:
> > >
> > > * Masami Hiramatsu <mhiramat@redhat.com> 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