LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pekka Paalanen <pq@iki.fi>
To: Arjan van de Ven <arjan@infradead.org>, Ingo Molnar <mingo@elte.hu>
Cc: Harvey Harrison <harvey.harrison@gmail.com>,
	linux-kernel@vger.kernel.org, Jan Beulich <jbeulich@novell.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Pavel Roskin <proski@gnu.org>,
	pq@iki.fi
Subject: Re: [PATCH v2] x86: Add a list for custom page fault handlers.
Date: Sun, 3 Feb 2008 08:55:22 +0200	[thread overview]
Message-ID: <20080203085522.2b63e15b@daedalus.pq.iki.fi> (raw)
In-Reply-To: <20080131081507.5edcde10@laptopd505.fenrus.org>

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

> On Thu, 31 Jan 2008 18:02:53 +0200
> Pekka Paalanen <pq@iki.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/

  reply	other threads:[~2008-02-03  6:55 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-27 16:52 [PATCH] " Pekka Paalanen
2008-01-27 17:55 ` [RFC PATCH] x86: mmiotrace - trace memory mapped IO Pekka Paalanen
2008-01-30 22:39   ` Pekka Paalanen
2008-01-27 19:29 ` [PATCH] x86: Add a list for custom page fault handlers Ingo Molnar
2008-01-27 21:03 ` Peter Zijlstra
2008-01-30  2:28 ` Harvey Harrison
2008-01-30  2:34   ` Harvey Harrison
2008-01-30 18:08     ` Pekka Paalanen
2008-01-31 15:07       ` Ingo Molnar
2008-01-31 16:02         ` [PATCH v2] " Pekka Paalanen
2008-01-31 16:15           ` Arjan van de Ven
2008-02-03  6:55             ` Pekka Paalanen [this message]
2008-02-03  7:03               ` Ingo Molnar
2008-02-03 21:40                 ` Pekka Paalanen
2008-02-05 20:28                 ` [PATCH 1/4] x86 mmiotrace: use lookup_address() Pekka Paalanen
2008-02-05 20:30                   ` [PATCH 2/4] x86 mmiotrace: fix relay-buffer-full flag for SMP Pekka Paalanen
2008-02-05 20:44                     ` Eric Dumazet
2008-02-05 21:14                       ` Pekka Paalanen
2008-02-05 21:35                         ` Eric Dumazet
2008-02-09 17:53                           ` [PATCH] x86 mmiotrace: Use percpu instead of arrays Pekka Paalanen
2008-02-05 20:31                   ` [PATCH 3/4] x86 mmiotrace: comment about user space ABI Pekka Paalanen
2008-02-05 20:39                   ` [PATCH 4/4] x86 mmiotrace: move files into arch/x86/mm/ Pekka Paalanen
2008-02-06  3:02                     ` Randy Dunlap
2008-02-09 11:21                       ` Pekka Paalanen
2008-02-07 12:53                     ` Ingo Molnar
2008-02-07 12:56                       ` Christoph Hellwig
2008-02-09 17:52                         ` [RFC PATCH] x86: explicit call to mmiotrace in do_page_fault() Pekka Paalanen
2008-02-09 18:01                           ` Arjan van de Ven
2008-02-09 18:23                             ` Pekka Paalanen
2008-02-09 18:56                               ` Pekka Enberg
2008-02-09 19:11                                 ` Pekka Paalanen
2008-02-09 19:19                                   ` Pekka Enberg
2008-02-09 18:39                             ` Peter Zijlstra
2008-02-09 18:39                           ` Peter Zijlstra
2008-02-10 18:05                             ` [RFC PATCH v2] " Pekka Paalanen
2008-02-11  2:12                               ` Pavel Roskin
2008-02-11 18:04                                 ` Pekka Paalanen
2008-02-06  5:00                   ` [PATCH 1/4] x86 mmiotrace: use lookup_address() Christoph Hellwig
2008-02-07 12:52                     ` Ingo Molnar
2008-01-31 16:16           ` [RFC PATCH v2] x86: mmiotrace - trace memory mapped IO Pekka Paalanen
2008-01-31 16:29             ` Arjan van de Ven
2008-02-03  7:21               ` Pekka Paalanen
2008-01-30 18:20 ` [PATCH] x86: Add a list for custom page fault handlers Arjan van de Ven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080203085522.2b63e15b@daedalus.pq.iki.fi \
    --to=pq@iki.fi \
    --cc=a.p.zijlstra@chello.nl \
    --cc=arjan@infradead.org \
    --cc=harvey.harrison@gmail.com \
    --cc=jbeulich@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=proski@gnu.org \
    --subject='Re: [PATCH v2] x86: Add a list for custom page fault handlers.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).