Replacement for page fault notifiers?

Replacement for page fault notifiers?

[Pekka Paalanen]

Hello,

the commit "x86: optimize page faults like all other achitectures and
kill notifier cruft" by Christoph Hellwig removed the page fault
notifiers (e.g. register_page_fault_notifier()) completely in 2.6.24.
Mmio-trace [1] is using them for tracking memory mapped IO, and also
other people have mentioned they need it [2,3].

This is roughly how I understand mmio-trace is working. In a loadable
kernel module calls to __ioremap() are diverted to an mmio-trace
specific function. The function calls the real __ioremap() and marks
the returned pages as not present. Access to the mapped IO memory
causes a page fault. Mmio-trace's page fault handler marks the page
present, records the event, single-steps the faulting instruction, and
finally marks the page as not present again.

- Is there anything coming to replace register_page_fault_notifier()?
- Has someone found another way to accomplish the same without patching
the kernel?
- Should I start writing a patch to bring back the notifiers, based on
the commit that removed them?

People started to complain to me that mmio-trace does not compile for
2.6.24. At least the Nouveau project is a regular user of mmio-trace
for reverse-engineering, and the tool should be easy to use. Therefore
I would prefer not to force casual contributors to patch their kernels
as it might be too much for some. I am hoping that 2.6.25, if not
2.6.24, would contain a mechanism to accomplish what I need.

I am not a subscriber, so please keep me in CC. Also I CC'd the people
who wrote about the same issue in the past.

Thanks.


[1] http://nouveau.freedesktop.org/wiki/MmioTrace
[2] http://marc.info/?l=linux-kernel&m=119540769014745&w=2
[3] http://article.gmane.org/gmane.linux.kernel/609521

-- 
Pekka Paalanen
http://www.iki.fi/pq/

Re: Replacement for page fault notifiers?

[Christoph Hellwig]

On Tue, Jan 08, 2008 at 09:06:38PM +0200, Pekka Paalanen wrote:
> - Is there anything coming to replace register_page_fault_notifier()?

no.

> - Has someone found another way to accomplish the same without patching
> the kernel?

no.

> - Should I start writing a patch to bring back the notifiers, based on
> the commit that removed them?

no.

> People started to complain to me that mmio-trace does not compile for
> 2.6.24. At least the Nouveau project is a regular user of mmio-trace
> for reverse-engineering, and the tool should be easy to use. Therefore
> I would prefer not to force casual contributors to patch their kernels
> as it might be too much for some. I am hoping that 2.6.25, if not
> 2.6.24, would contain a mechanism to accomplish what I need.

Please make mmio tracing generic enough so it works for every device and
submit it for inclusion as a config option.

Re: Replacement for page fault notifiers?

[Dave Airlie]

On Jan 9, 2008 6:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jan 08, 2008 at 09:06:38PM +0200, Pekka Paalanen wrote:
> > - Is there anything coming to replace register_page_fault_notifier()?
>
> no.
>
> > - Has someone found another way to accomplish the same without patching
> > the kernel?
>
> no.
>
> > - Should I start writing a patch to bring back the notifiers, based on
> > the commit that removed them?
>
> no.
>
> > People started to complain to me that mmio-trace does not compile for
> > 2.6.24. At least the Nouveau project is a regular user of mmio-trace
> > for reverse-engineering, and the tool should be easy to use. Therefore
> > I would prefer not to force casual contributors to patch their kernels
> > as it might be too much for some. I am hoping that 2.6.25, if not
> > 2.6.24, would contain a mechanism to accomplish what I need.
>
> Please make mmio tracing generic enough so it works for every device and
> submit it for inclusion as a config option.
>

Christoph,

it is generic code, it works by patching the module you load and is in
no way nvidia specific, I think we should revert your chance
as the notifiers have a valid user and one it has been planned to upstream..

Dave.

Re: Replacement for page fault notifiers?

[Benjamin Herrenschmidt]

> Christoph,
> 
> it is generic code, it works by patching the module you load and is in
> no way nvidia specific, I think we should revert your chance
> as the notifiers have a valid user and one it has been planned to upstream..

Agreed, it looks like a sane enough user of page fault notifiers, it's
going to be upstream, and it's useful to fight evil binary drivers so
that sounds like a good justification to bring those back in :-)

Cheers,
Ben.

Re: Replacement for page fault notifiers?

[Pavel Roskin]

On Wed, 2008-01-09 at 11:26 +1100, Benjamin Herrenschmidt wrote:
> > Christoph,
> > 
> > it is generic code, it works by patching the module you load and is in
> > no way nvidia specific, I think we should revert your chance
> > as the notifiers have a valid user and one it has been planned to upstream..
> 
> Agreed, it looks like a sane enough user of page fault notifiers, it's
> going to be upstream, and it's useful to fight evil binary drivers so
> that sounds like a good justification to bring those back in :-)

The fault notifiers are used in a special version of MadWifi (Atheros
wireless driver with some non-free parts) to figure out which registers
are being accessed.  The tracing capability has been essential for the
development of the free replacement, ath5k.

Considering that ath5k is being developed in the kernel tree, whereas
madwifi-trace needs 2.6.23, one needs different kernels for tracing and
development.  Lifting this requirement would be a time saver for the the
developers of ath5k.  And once 2.6.24 is released, ath5k developers will
be in the position to ask users to downgrade their kernels to get traces
for their cards.  That's something I'd like to avoid.

I looked at ways to implement the page fault handler using kprobes, but
I could not find a simple solution.  The problem is that kprobes uses
the fault notifier for its own needs.  I'm still hopeful that somebody
with a better understanding of kprobes will be able to implement fault
notification without patching the kernel.  But if no such solution is
found, I would also support reverting the patch that removed fault
notifiers on i386.

-- 
Regards,
Pavel Roskin

Re: Replacement for page fault notifiers?

[Christoph Hellwig]

On Wed, Jan 09, 2008 at 01:18:44PM -0500, Pavel Roskin wrote:
> notification without patching the kernel.  But if no such solution is
> found, I would also support reverting the patch that removed fault
> notifiers on i386.

With your fixation on not patching the kernel you sound like a windows
developer.  There is no problem with patching the kernel, but the best
patching of that kernel is that which happens upstream so please folks
start submitting an mmiotrace variant for kernel inclusion.

Then again I don't quite get why you absolutely want to track pagefaults
anyway.  Just hooking into ioremap and read*/write* and iomap +
ioread*/iowrite* would be much easier and also a lot faster.  It would
also allow adding a nice sysfs attribute to enable this per-device.

Re: Replacement for page fault notifiers?

[Benjamin Herrenschmidt]

On Wed, 2008-01-09 at 18:21 +0000, Christoph Hellwig wrote:
> On Wed, Jan 09, 2008 at 01:18:44PM -0500, Pavel Roskin wrote:
> > notification without patching the kernel.  But if no such solution is
> > found, I would also support reverting the patch that removed fault
> > notifiers on i386.
> 
> With your fixation on not patching the kernel you sound like a windows
> developer.  There is no problem with patching the kernel, but the best
> patching of that kernel is that which happens upstream so please folks
> start submitting an mmiotrace variant for kernel inclusion.
> 
> Then again I don't quite get why you absolutely want to track pagefaults
> anyway.  Just hooking into ioremap and read*/write* and iomap +
> ioread*/iowrite* would be much easier and also a lot faster.  It would
> also allow adding a nice sysfs attribute to enable this per-device.

Because binary drivers don't use ioread/iowrite/readX/writeX (or if they
do, they've long been inlined in the blob). The only solution is to map
things as non-accessible and trap the page faults.

It's a sane thing to do, Christoph, I don't think it's a unreasonable
request to put the hooks back in.

Cheers,
Ben.

Re: Replacement for page fault notifiers?

[Christoph Hellwig]

On Thu, Jan 10, 2008 at 06:58:23AM +1100, Benjamin Herrenschmidt wrote:
> It's a sane thing to do, Christoph, I don't think it's a unreasonable
> request to put the hooks back in.

As said a few times before there's simply no way we're going to put
exactly that crap back.  For one the patch removed a whole lot of
crud from the kprobes code that simply isn't going to come back just
because there are some pagefault notifiers.  Second the page fault
notifiers were horribly implemented and quite inefficient.  And third
we're not going to put something in just for out of tree code.

Please submit the mmiotrace for 2.6.25 and we'll find some way to make
it work.

Re: Replacement for page fault notifiers?

[Arjan van de Ven]

On Wed, 9 Jan 2008 20:22:54 +0000
Christoph Hellwig <hch@infradead.org> wrote:

> On Thu, Jan 10, 2008 at 06:58:23AM +1100, Benjamin Herrenschmidt
> wrote:
> > It's a sane thing to do, Christoph, I don't think it's a
> > unreasonable request to put the hooks back in.
> 
> As said a few times before there's simply no way we're going to put
> exactly that crap back.  For one the patch removed a whole lot of
> crud from the kprobes code that simply isn't going to come back just
> because there are some pagefault notifiers.  Second the page fault
> notifiers were horribly implemented and quite inefficient.  And third
> we're not going to put something in just for out of tree code.
> 

I'm btw all in favor of making mmio tracing full fledged kernel infrastructure.
This doesn't mean "notifier" imo; this means a real flag in the struct page,
and then the page fault code can do

if (page->flags & FLAG_MMIO_TRACED)
	mmio_trace(page, regs, whatever..);

(probably surrounded by a CONFIG_ ifdef)
THis is a TON lighter than a notifier chain, and actually what you want,
you don't really want a notifier, you want a call back when a special kind of
page is touched.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: Replacement for page fault notifiers?

[Andi Kleen]

Arjan van de Ven <arjan@infradead.org> writes:
>
> I'm btw all in favor of making mmio tracing full fledged kernel infrastructure.
> This doesn't mean "notifier" imo; this means a real flag in the struct page,
> and then the page fault code can do
>
> if (page->flags & FLAG_MMIO_TRACED)
> 	mmio_trace(page, regs, whatever..);
>
> (probably surrounded by a CONFIG_ ifdef)
> THis is a TON lighter than a notifier chain, and actually what you want,
> you don't really want a notifier, you want a call back when a special kind of
> page is touched.

That would assume that your mmio area has a struct page. In most PCs 
the ones in the PCI hole don't

-Andi

Re: Replacement for page fault notifiers?

[Arjan van de Ven]

On Thu, 10 Jan 2008 01:47:16 +0100
Andi Kleen <andi@firstfloor.org> wrote:

> Arjan van de Ven <arjan@infradead.org> writes:
> >
> > I'm btw all in favor of making mmio tracing full fledged kernel
> > infrastructure. This doesn't mean "notifier" imo; this means a real
> > flag in the struct page, and then the page fault code can do
> >
> > if (page->flags & FLAG_MMIO_TRACED)
> > 	mmio_trace(page, regs, whatever..);
> >
> > (probably surrounded by a CONFIG_ ifdef)
> > THis is a TON lighter than a notifier chain, and actually what you
> > want, you don't really want a notifier, you want a call back when a
> > special kind of page is touched.
> 
> That would assume that your mmio area has a struct page. In most PCs 
> the ones in the PCI hole don't
> 

so you also call the function for all traps on pages without struct page;
that should be extremely rare anyway, and the mmio_trace code can then 
look the page up.

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: Replacement for page fault notifiers?

[Benjamin Herrenschmidt]

On Wed, 2008-01-09 at 16:55 -0800, Arjan van de Ven wrote:
> > That would assume that your mmio area has a struct page. In most
> PCs 
> > the ones in the PCI hole don't
> > 
> 
> so you also call the function for all traps on pages without struct
> page;
> that should be extremely rare anyway, and the mmio_trace code can
> then 
> look the page up.

Not that rare if you use a modern DRI :-)

In fact, the thing here is that it's mostly kernel mappings though, not
user mappings. So we never get there, we die before we even reach
generic code most of the time iirc.

The whole ioremap stuff is very platform specific, not everybody even
uses the code in mm/vmalloc.c for it, so at this stage, I see no other
option but a hook in do_page_fault().

Cheers,
Ben.

Re: Replacement for page fault notifiers?

[Andi Kleen]

On Wed, Jan 09, 2008 at 04:55:15PM -0800, Arjan van de Ven wrote:
> On Thu, 10 Jan 2008 01:47:16 +0100
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > Arjan van de Ven <arjan@infradead.org> writes:
> > >
> > > I'm btw all in favor of making mmio tracing full fledged kernel
> > > infrastructure. This doesn't mean "notifier" imo; this means a real
> > > flag in the struct page, and then the page fault code can do
> > >
> > > if (page->flags & FLAG_MMIO_TRACED)
> > > 	mmio_trace(page, regs, whatever..);
> > >
> > > (probably surrounded by a CONFIG_ ifdef)
> > > THis is a TON lighter than a notifier chain, and actually what you
> > > want, you don't really want a notifier, you want a call back when a
> > > special kind of page is touched.
> > 
> > That would assume that your mmio area has a struct page. In most PCs 
> > the ones in the PCI hole don't
> > 
> 
> so you also call the function for all traps on pages without struct page;
> that should be extremely rare anyway, and the mmio_trace code can then 
> look the page up.

But there's really no page to look up. These are typically !pfn_valid()

You would need to create an own backing infrastructure. Perhaps you can reuse
some of that from the future PAT code, but integrating that into the page fault
will be probably ugly.

-Andi

Re: Replacement for page fault notifiers?

[Benjamin Herrenschmidt]

> I'm btw all in favor of making mmio tracing full fledged kernel infrastructure.
> This doesn't mean "notifier" imo; this means a real flag in the struct page,
> and then the page fault code can do
> 
> if (page->flags & FLAG_MMIO_TRACED)
> 	mmio_trace(page, regs, whatever..);
> 
> (probably surrounded by a CONFIG_ ifdef)
> THis is a TON lighter than a notifier chain, and actually what you want,
> you don't really want a notifier, you want a call back when a special kind of
> page is touched.
> 

Except that MMIO has no struct page's...

Cheers,
Ben.

Re: Replacement for page fault notifiers?

[Matt Mackall]

On Wed, 2008-01-09 at 16:42 -0800, Arjan van de Ven wrote:
> On Wed, 9 Jan 2008 20:22:54 +0000
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Thu, Jan 10, 2008 at 06:58:23AM +1100, Benjamin Herrenschmidt
> > wrote:
> > > It's a sane thing to do, Christoph, I don't think it's a
> > > unreasonable request to put the hooks back in.
> > 
> > As said a few times before there's simply no way we're going to put
> > exactly that crap back.  For one the patch removed a whole lot of
> > crud from the kprobes code that simply isn't going to come back just
> > because there are some pagefault notifiers.  Second the page fault
> > notifiers were horribly implemented and quite inefficient.  And third
> > we're not going to put something in just for out of tree code.
> > 
> 
> I'm btw all in favor of making mmio tracing full fledged kernel infrastructure.
> This doesn't mean "notifier" imo; this means a real flag in the struct page,
> and then the page fault code can do
> 
> if (page->flags & FLAG_MMIO_TRACED)
> 	mmio_trace(page, regs, whatever..);

That makes it way too easy for drivers of questionable legality to just
clear that bit. Also, we've got a shortage of page bits, etc.

-- 
Mathematics is the supreme nostalgia of our time.

Re: Replacement for page fault notifiers?

[Pavel Roskin]

On Wed, 2008-01-09 at 20:03 -0600, Matt Mackall wrote:

> That makes it way too easy for drivers of questionable legality to just
> clear that bit. Also, we've got a shortage of page bits, etc.

If we ever have this problem, the bit can be changed in the kernel to
fool those drivers (I hope the shortage is not so dire that there will
be no more bits left).

I don't think evil drivers should be a problem per se.  There are
existing non-free drivers that still need to be traced over and over
again.  I guess ndiswrapper could use tracing for Windows drivers that
don't know anything about Linux page flags.

Last but not least, mmiotrace should be useful for free drivers in the
first place to have a legitimate reason to be in the kernel.

-- 
Regards,
Pavel Roskin

Re: Replacement for page fault notifiers?

[Matt Mackall]

On Wed, 2008-01-09 at 21:21 -0500, Pavel Roskin wrote:
> On Wed, 2008-01-09 at 20:03 -0600, Matt Mackall wrote:
> 
> > That makes it way too easy for drivers of questionable legality to just
> > clear that bit. Also, we've got a shortage of page bits, etc.
> 
> If we ever have this problem, the bit can be changed in the kernel to
> fool those drivers (I hope the shortage is not so dire that there will
> be no more bits left).

That's what we call an arms race. And yes, page bits are close to
running out on 32-bit.

> I don't think evil drivers should be a problem per se.  There are
> existing non-free drivers that still need to be traced over and over
> again.  I guess ndiswrapper could use tracing for Windows drivers that
> don't know anything about Linux page flags.
> 
> Last but not least, mmiotrace should be useful for free drivers in the
> first place to have a legitimate reason to be in the kernel.

I definitely agree that tracing MMIO is an extremely useful feature.
I've implemented it myself once already for 2.4.

-- 
Mathematics is the supreme nostalgia of our time.

Re: Replacement for page fault notifiers?

[Pekka Paalanen]

On Wed, 9 Jan 2008 20:22:54 +0000
Christoph Hellwig <hch@infradead.org> wrote:

> Please submit the mmiotrace for 2.6.25 and we'll find some way to make
> it work.

Fair enough. Thanks to all for the encouraging words. I will start to
work towards sending mmio-trace to kernel.

We'll see how far I get before 2.6.25 closes. I'm new to all this and
will need help, especially in making mmio-trace SMP-safe, for instance.

-- 
Pekka Paalanen
http://www.iki.fi/pq/

Re: Replacement for page fault notifiers?

[Pavel Roskin]

On Thu, 2008-01-10 at 06:58 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2008-01-09 at 18:21 +0000, Christoph Hellwig wrote:
> > On Wed, Jan 09, 2008 at 01:18:44PM -0500, Pavel Roskin wrote:
> > > notification without patching the kernel.  But if no such solution is
> > > found, I would also support reverting the patch that removed fault
> > > notifiers on i386.
> > 
> > With your fixation on not patching the kernel you sound like a windows
> > developer.

I just assumed (wrongly, as it seems) that the API change didn't remove
any useful functionality.

Also, I don't want to ask users with unusual hardware to patch their
kernels to take the trace.

> > There is no problem with patching the kernel, but the best
> > patching of that kernel is that which happens upstream so please folks
> > start submitting an mmiotrace variant for kernel inclusion.

The problem is that mmiotrace only makes sense in combination with
non-free drivers, and I'm not sure it would be welcome in the kernel
even if you say so.

The hooks for page faults could be useful for something (debugging, some
non-traditional swap implementations), but I don't want to invent
reasons why somebody else might need them.

> > Then again I don't quite get why you absolutely want to track pagefaults
> > anyway.  Just hooking into ioremap and read*/write* and iomap +
> > ioread*/iowrite* would be much easier and also a lot faster.  It would
> > also allow adding a nice sysfs attribute to enable this per-device.
> 
> Because binary drivers don't use ioread/iowrite/readX/writeX (or if they
> do, they've long been inlined in the blob). The only solution is to map
> things as non-accessible and trap the page faults.

Exactly.  In MadWifi, they are inlined on i386 and x86_64, and I cannot
ask every user with an unsupported card to get a Mac.

> It's a sane thing to do, Christoph, I don't think it's a unreasonable
> request to put the hooks back in.

Seconded.

-- 
Regards,
Pavel Roskin

Re: Replacement for page fault notifiers?

[Christoph Hellwig]

On Wed, Jan 09, 2008 at 03:24:16PM -0500, Pavel Roskin wrote:
> I just assumed (wrongly, as it seems) that the API change didn't remove
> any useful functionality.

No, this was exactly the correct assumption.  Out of tree modules simply
don't count.

> The problem is that mmiotrace only makes sense in combination with
> non-free drivers, and I'm not sure it would be welcome in the kernel
> even if you say so.

It can trace every driver in theory although it's of course not
really interesting for free drivers.  But it's not actually
functionality used by the drivers but functionlity to trace the drivers
so it's quite different anyway.

> Exactly.  In MadWifi, they are inlined on i386 and x86_64, and I cannot
> ask every user with an unsupported card to get a Mac.

Maybe it's time to do some simple xoring in the ioremap return value
to force them not to do such stupid things..

Re: Replacement for page fault notifiers?

[Pavel Roskin]

On Wed, 2008-01-09 at 20:26 +0000, Christoph Hellwig wrote:
> On Wed, Jan 09, 2008 at 03:24:16PM -0500, Pavel Roskin wrote:
> > I just assumed (wrongly, as it seems) that the API change didn't remove
> > any useful functionality.
> 
> No, this was exactly the correct assumption.  Out of tree modules simply
> don't count.
> 
> > The problem is that mmiotrace only makes sense in combination with
> > non-free drivers, and I'm not sure it would be welcome in the kernel
> > even if you say so.
> 
> It can trace every driver in theory although it's of course not
> really interesting for free drivers.  But it's not actually
> functionality used by the drivers but functionlity to trace the drivers
> so it's quite different anyway.

OK, that's reassuring.  Maybe it could be used for profiling or to find
bugs that are not obvious otherwise.

-- 
Regards,
Pavel Roskin

Re: Replacement for page fault notifiers?

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Wed, 09 Jan 2008 20:26:44 GMT, Christoph Hellwig said:

> > Exactly.  In MadWifi, they are inlined on i386 and x86_64, and I cannot
> > ask every user with an unsupported card to get a Mac.
> 
> Maybe it's time to do some simple xoring in the ioremap return value
> to force them not to do such stupid things..

If an open-source driver did exactly the same thing, would it still be
stupid?  If so, what should it be doing instead?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]