This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [BUGFIX][PATCH -rc/-mm] prevent kprobes from catching spurious page faults


* 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.

	Ingo


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]