Message-ID: <20080203085522.2b63e15b@daedalus.pq.iki.fi>
Date:	Sun, 3 Feb 2008 08:55:22 +0200
From:	Pekka Paalanen <pq@....fi>
To:	Arjan van de Ven <arjan@...radead.org>, Ingo Molnar <mingo@...e.hu>
Cc:	Harvey Harrison <harvey.harrison@...il.com>,
	linux-kernel@...r.kernel.org, Jan Beulich <jbeulich@...ell.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Pavel Roskin <proski@....org>, pq@....fi
Subject: Re: [PATCH v2] x86: Add a list for custom page fault handlers.

On Thu, 31 Jan 2008 08:15:07 -0800
Arjan van de Ven <arjan@...radead.org> wrote:

> On Thu, 31 Jan 2008 18:02:53 +0200
> Pekka Paalanen <pq@....fi> wrote:
> > 
> > Very well, first the revised custom page fault handler patch.
> > Changes since the previous submit:
> > - use spin_lock_irqsave instead of spin_lock
> > - Harvey Harrison's clean-up with the #ifdefs
> > - handler call site moved earlier
> > - remove sync RCU call

> it's overhead that's rarely used, and worse, it suffers from the LSM disease:
> it's a hook without visibility into the callers. Esp if there's only 1 user, a direct call
> is not only faster, it also shows you what is going on when you're reading the code.
> THe page fault stuff is tricky enough as it is without any "call arbitrary thing" hooks into
> it

I understand your point.

> > This is most convinient for mmiotrace as it is meant to be a module.
> 
> that's not per se mutually exclusive, there's other cases like this already
> (but even then, I don't want to go through hoops to make mmiotracing a module;
> for me I can totally see it becoming a full mature member of the kernel, and if
> that means a few things have to be changed in the normal kernel, or infrastructure
> added, I'm totally ok with that)

I have two reasons why I'd like to let it be a module:
- it's "broken", the relay buffers are cleared on module unload
- it's a lot easier to push updated version for testing to people

Ok, the first one is just a silly excuse, but the second one is to
avoid forcing non-developer users to patch their kernels. This is a
much needed tool for the Nouveau project even in it's current form.

> > I'm also using this as an excuse to let other people to get into the
> > page fault handler with their out-of-tree-today modules.
> 
> this is exactly the point.. it becomes a totally opaque thing, a random hook.

And this was the third reason I wanted a module, but I understand this
is an unacceptable justification.

What are these "othes cases" I could look at as an example of a hook,
that is meant for a single specific module? I do not understand what
could prevent out-of-tree modules from using the hook if my module can
use it (and my module is not using it at the same time).

What is the verdict?
Will the custom page fault handler patch v2 get merged as it is,
do I need to modify it (how?), do I need to rewrite it based on
some example (which?), or do I have to drop the whole idea of
mmiotrace being kernel module and make it work as built-in?

In the case of making it a built-in, I do not think I have any
chance to get it done for 2.6.25.

There's one more option. I could make kmmio.c things as builtin,
and keep the mmio-mod.c as the module. This is how things were in
the very beginning of mmiotrace, when the page fault notifiers did
not exist. This could be an acceptable compromise, no?
But I don't think I will get that ready for 2.6.25, either.

What about merging the custom page fault handler patch now, and
removing it for 2.6.26?

Looks like Beulich and Roskin will be disappointed anyway. They
were hoping for a usable page fault hook.

-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists