public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: Mathieu Desnoyers <compudj@krystal.dyndns.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	Ingo Molnar <mingo@elte.hu>,
	akpm@osdl.org, "H. Peter Anvin" <hpa@zytor.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	"Frank Ch. Eigler" <fche@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] x86 NMI-safe INT3 and Page Fault (v3)
Date: Thu, 17 Apr 2008 18:45:38 +0200	[thread overview]
Message-ID: <48077EB2.9030009@firstfloor.org> (raw)
In-Reply-To: <20080417162932.GA23351@Krystal>

Mathieu Desnoyers wrote:
> * Jeremy Fitzhardinge (jeremy@goop.org) wrote:
>> Mathieu Desnoyers wrote:
>>> "This way lies madness. Don't go there."
>>>   
>> It is a large amount of... stuff.  This immediate values thing makes a big 
>> improvement then?
>>
> 
> As ingo said : the nmi-safe traps and exception is not only usefu lto
> immediate values, but also to oprofile. 

How is it useful to oprofile?

> On top of that, the LTTng kernel
> tracer has to write into vmalloc'd memory, so it's required there too.

All this effort changing really critical (and also fragile) code paths
used all the time is to handle setting markers into NMI functions. Or
actually the special case of setting markers in there that access
vmalloc() without calling vmalloc_sync().

NMI are maybe 5-6 functions all over the kernel.

I just don't think it makes any sense to put markers in there.
It is a really small part of the kernel the kernel that is unlikely
to be really useful for anybody. You should rather first solve the
problem of tracing the other 99.999999% of the kernel properly.

And then you could actually set the markers in there if you're
crazy enough, just call vmalloc_sync().

Mathieu argued earlier that markers should be set everywhere but
that is also bogus because there is enough other code where
you cannot set them either (one example would be early boot code[1])

And to do anything in NMI context you cannot use any locks so you would
have to write all data structures used by the markers lock less. I did
that for the the new mce code, but it's a really painful and bug prone
experience that I cannot really recommend to anybody.

And then NMIs (and machine checks) are a really obscure case, very
rarely used.

I think the right way is just to say that you cannot set markers
into NMI and machine check. Even with this patch it is highly unlikely
the resulting code will be correct anyways. Actually you could probably
set them without the patch with some effort (like calling vmalloc_sync),
but for the basic reasons mentioned above (lock less code is really
hard, nmi type functions are less than hundred lines in the millions
of kernel LOCs) it is just a very very bad idea.

-Andi


[1] Now that I mentioned it I still have enough faith to assume nobody
will be crazy enough to come up with some horrible hack to set markers
in early boot code too. But after seeing this patchkit ending up in a
git tree I'm not sure.

  reply	other threads:[~2008-04-17 16:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080414230344.GA16061@Krystal>
2008-04-14 23:05 ` [RFC PATCH] x86 NMI-safe INT3 and Page Fault (v2) Mathieu Desnoyers
2008-04-16 13:06   ` Ingo Molnar
2008-04-16 13:47     ` [TEST PATCH] Test NMI kprobe modules Mathieu Desnoyers
2008-04-16 14:34       ` Ingo Molnar
2008-04-16 14:54         ` Mathieu Desnoyers
2008-04-16 15:10     ` [RFC PATCH] x86 NMI-safe INT3 and Page Fault (v2) Ingo Molnar
2008-04-16 15:18       ` H. Peter Anvin
2008-04-16 15:37       ` Mathieu Desnoyers
2008-04-16 16:03       ` Jeremy Fitzhardinge
2008-04-18  0:48         ` Mathieu Desnoyers
2008-04-18  9:49           ` Jeremy Fitzhardinge
2008-04-16 16:28       ` [RFC PATCH] x86 NMI-safe INT3 and Page Fault (v3) Mathieu Desnoyers
2008-04-16 17:57         ` Jeremy Fitzhardinge
2008-04-17 16:29           ` Mathieu Desnoyers
2008-04-17 16:45             ` Andi Kleen [this message]
2008-04-18  0:05               ` Mathieu Desnoyers
2008-04-18  8:27                 ` Andi Kleen
2008-04-19 21:18                   ` Mathieu Desnoyers
2008-04-20 12:58                     ` Andi Kleen

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=48077EB2.9030009@firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=akpm@osdl.org \
    --cc=compudj@krystal.dyndns.org \
    --cc=fche@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox