[PATCH -tip perf/probes 00/10] x86 insn decoder bugfixes

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Mon, Nov 02, 2009 at 04:16:25PM -0500, Masami Hiramatsu wrote:
> Masami Hiramatsu wrote:
>> Ingo Molnar wrote:
>>> What we want here is two fold:
>>>
>>> - enable kprobes event support when perf events is enabled and kprobes
>>> is enabled. We dont want another config option for it.
>>
>> Sure, at least that combination should enable kprobe-tracer forcibly.
>
> Hmm, someone may not want to enables kprobe-tracer. Perhaps,
> "default y if (EVENT_PROFILE)" is enough, isn't it?
>

I guess it should be sufficient yeah. We want to strongly recommend
the kprobe events if we have enabled perf, but we don't want to force
it.

Also in this case we need a verbose runtime report of the lack of this
tracer in debugfs from perf probe if needed.

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

Masami Hiramatsu · External Since: Jan 13, 2009 Posts: 88

Frederic Weisbecker wrote:
> On Mon, Nov 02, 2009 at 04:16:25PM -0500, Masami Hiramatsu wrote:
>> Masami Hiramatsu wrote:
>>> Ingo Molnar wrote:
>>>> What we want here is two fold:
>>>>
>>>> - enable kprobes event support when perf events is enabled and kprobes
>>>> is enabled. We dont want another config option for it.
>>>
>>> Sure, at least that combination should enable kprobe-tracer forcibly.
>>
>> Hmm, someone may not want to enables kprobe-tracer. Perhaps,
>> "default y if (EVENT_PROFILE)" is enough, isn't it?
>>
>
>
> I guess it should be sufficient yeah. We want to strongly recommend
> the kprobe events if we have enabled perf, but we don't want to force
> it.
>
> Also in this case we need a verbose runtime report of the lack of this
> tracer in debugfs from perf probe if needed.

Sure, error message should be changed as warning user to enable
CONFIG_KPROBE_TRACER Smile

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat DeleteThis @redhat.com

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

Masami Hiramatsu · External Since: Jan 13, 2009 Posts: 88

Masami Hiramatsu wrote:
> Masami Hiramatsu wrote:
>> Ingo Molnar wrote:
>>> What we want here is two fold:
>>>
>>> - enable kprobes event support when perf events is enabled and kprobes
>>> is enabled. We dont want another config option for it.
>>
>> Sure, at least that combination should enable kprobe-tracer forcibly.
>
> Hmm, someone may not want to enables kprobe-tracer. Perhaps,
> "default y if (EVENT_PROFILE)" is enough, isn't it?

Oops, this causes recursive dependency error Sad

kernel/trace/Kconfig:90:error: found recursive dependency: TRACING ->
EVENT_TRACING -> EVENT_PROFILE -> KPROBE_TRACER -> GENERIC_TRACER -> TRACING

At this time, this kind of weak dependency may not supported by Kbuild yet.

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat.RemoveThis@redhat.com

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

Ingo Molnar · External Since: May 15, 2006 Posts: 3112

* Roland McGrath <roland.RemoveThis@redhat.com> wrote:

> > Thirdly, i think we should expose the build-id of the kernel and the
> > path to the vmlinux (and modules) via /proc/build-id or so. That way
> > tooling can find the vmlinux file and can validate that it matches
> > the kernel's signature. (maybe include a file date as well - that's
> > a faster check than a full build-id checksum, especially for large
> > kernels)
>
> You seem to be confused about what build IDs are. There is no
> "checksum validation". Once the bits are stored there is no
> calculation ever done again, only exact comparison with an expected
> build ID bitstring. The size of an object file is immaterial.
>
> As Frank mentioned, the kernel's and modules' allocated ELF notes (and
> thus build IDs) are already exposed in /sys. Tools like "eu-unstrip
> -nk" use this information today.

Ah, i didnt realize we link with --build-id already, unconditonally,
since v2.6.23 (if ld supports it):

|
| From 18991197b4b588255ccabf472ebc84db7b66a19c Mon Sep 17 00:00:00 2001
| From: Roland McGrath <roland.RemoveThis@redhat.com>
| Date: Thu, 19 Jul 2007 01:48:40 -0700
| Subject: [PATCH] Use --build-id ld option
|
| This change passes the --build-id when linking the kernel and when
| linking modules, if ld supports it. This is a new GNU ld option that
| synthesizes an ELF note section inside the read-only data. The note in
| this section contains unique identifying bits called the "build ID",
| which are generated so as to be different for any two linked ELF files
| that aren't identical.
|

So we have an SHA1 build-id already on the vmlinux and on modules, and
it's exposed in /sys/*/*/notes. Just have to make use of it in
tools/perf too.

The other useful addition i mentioned isnt implemented yet: to emit an
ELF note of the absolute path of the output directory the kernel was
built in as well. This information is not available right now, and it
would be a place to look in to search for the vmlinux and the modules.

What do you think? Also, if we do this, is there a standard way to name
it , or should i just pick a suitably new, Linux-specific name for that?

Thanks,

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

Ingo Molnar · External Since: May 15, 2006 Posts: 3112

* Masami Hiramatsu <mhiramat DeleteThis @redhat.com> wrote:

> Masami Hiramatsu wrote:
>> Masami Hiramatsu wrote:
>>> Ingo Molnar wrote:
>>>> What we want here is two fold:
>>>>
>>>> - enable kprobes event support when perf events is enabled and kprobes
>>>> is enabled. We dont want another config option for it.
>>>
>>> Sure, at least that combination should enable kprobe-tracer forcibly.
>>
>> Hmm, someone may not want to enables kprobe-tracer. Perhaps,
>> "default y if (EVENT_PROFILE)" is enough, isn't it?
>
> Oops, this causes recursive dependency error Sad

>
> kernel/trace/Kconfig:90:error: found recursive dependency: TRACING ->
> EVENT_TRACING -> EVENT_PROFILE -> KPROBE_TRACER -> GENERIC_TRACER -> TRACING

This dependency problem can be resolved by simply making it 'default y'
- the option itself depends on KPROBES already, which is default-off -
so no need to also make it depend on EVENT_PROFILE.

btw., it would be nice to re-name it to 'KPROBE_EVENTS'. If the probe
point is used as a count - like in the __switch_to example i cited -
there's no tracing going on at all.

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

Arnaldo Carvalho de Melo · External Since: Oct 29, 2009 Posts: 8

Em Tue, Nov 03, 2009 at 08:24:39AM +0100, Ingo Molnar escreveu:
> * Roland McGrath <roland RemoveThis @redhat.com> wrote:
> > As Frank mentioned, the kernel's and modules' allocated ELF notes (and
> > thus build IDs) are already exposed in /sys. Tools like "eu-unstrip
> > -nk" use this information today.
>
> Ah, i didnt realize we link with --build-id already, unconditonally,
> since v2.6.23 (if ld supports it):
>
> | From 18991197b4b588255ccabf472ebc84db7b66a19c Mon Sep 17 00:00:00 2001
> | Subject: [PATCH] Use --build-id ld option
>
> So we have an SHA1 build-id already on the vmlinux and on modules, and
> it's exposed in /sys/*/*/notes. Just have to make use of it in
> tools/perf too.

I wasn't aware this was done upstream, ass-umed that it was only on
kernel specfiles, will cook up a patch for consideration.

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

Masami Hiramatsu · External Since: Jan 13, 2009 Posts: 88

Ingo Molnar wrote:
> * Masami Hiramatsu<mhiramat.RemoveThis@redhat.com> wrote:
>> Masami Hiramatsu wrote:
>>> Masami Hiramatsu wrote:
>>>> Ingo Molnar wrote:
>>>>> What we want here is two fold:
>>>>>
>>>>> - enable kprobes event support when perf events is enabled and kprobes
>>>>> is enabled. We dont want another config option for it.
>>>>
>>>> Sure, at least that combination should enable kprobe-tracer forcibly.
>>>
>>> Hmm, someone may not want to enables kprobe-tracer. Perhaps,
>>> "default y if (EVENT_PROFILE)" is enough, isn't it?
>>
>> Oops, this causes recursive dependency error Sad

>>
>> kernel/trace/Kconfig:90:error: found recursive dependency: TRACING ->
>> EVENT_TRACING -> EVENT_PROFILE -> KPROBE_TRACER -> GENERIC_TRACER -> TRACING
>
> This dependency problem can be resolved by simply making it 'default y'
> - the option itself depends on KPROBES already, which is default-off -
> so no need to also make it depend on EVENT_PROFILE.

OK,

> btw., it would be nice to re-name it to 'KPROBE_EVENTS'. If the probe
> point is used as a count - like in the __switch_to example i cited -
> there's no tracing going on at all.

Sure, it's not a tracer anyway Smile

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat.RemoveThis@redhat.com

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