[Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 301 bytes --]

Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
over-instrumentation. This patch aligns x86_64 in this regard with i386
(and hopefully the other tracer-supporting archs too). It also gets rid
of related *_thunk indirections (I wonder if we cannot kill more of
them...).

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: cleanup-irqoff-instrumentation.patch --]
[-- Type: text/x-patch; name="cleanup-irqoff-instrumentation.patch", Size: 4525 bytes --]

---
 arch/x86_64/kernel/entry.S    |   26 ++++++++++++++++++--------
 arch/x86_64/kernel/ipipe.c    |   16 ----------------
 arch/x86_64/lib/thunk.S       |    5 -----
 include/asm-x86_64/irqflags.h |   15 ++++-----------
 4 files changed, 22 insertions(+), 40 deletions(-)

Index: linux-2.6.23.1-xeno_64/arch/x86_64/kernel/entry.S
===================================================================
--- linux-2.6.23.1-xeno_64.orig/arch/x86_64/kernel/entry.S
+++ linux-2.6.23.1-xeno_64/arch/x86_64/kernel/entry.S
@@ -542,16 +542,28 @@ END(stub_rt_sigreturn)
 	 */
 	TRACE_IRQS_OFF
 #ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-        leaq RIP-8(%rdi), %rbp
-        pushq %rdi
-        movq ORIG_RAX(%rdi), %rdi
+	leaq RIP-8(%rdi), %rbp	# make interrupted address show up in trace
+	pushq %rdi
+	movq ORIG_RAX(%rdi), %rdi	# IRQ number
+	notq %rdi			# ...is inverted, fix up
+	call ipipe_trace_begin
+	popq %rdi
+	popq %rbp
+	pushq %rbp
+
+	call \func
+
+	popq %rbp
+	pushq %rbp
+	movq 8-ARGOFFSET+ORIG_RAX(%rbp), %rdi
+	leaq 8-ARGOFFSET+RIP-8(%rbp), %rbp
 	notq %rdi
-        call ipipe_trace_begin_thunk
-        popq %rdi
+	call ipipe_trace_end
 	popq %rbp
 	pushq %rbp
-#endif
+#else
 	call \func
+#endif
 	.endm
 
 #ifdef CONFIG_IPIPE
@@ -602,7 +614,6 @@ ENTRY(common_interrupt)
 	jnz ret_from_intr
 	decl %gs:pda_irqcount
 	leaveq
-	IPIPE_TRACE_IRQS_ON
 	CFI_DEF_CFA_REGISTER	rsp
 	CFI_ADJUST_CFA_OFFSET	-8
 	testl $3,CS-ARGOFFSET(%rsp)
@@ -734,7 +745,6 @@ END(common_interrupt)
 	jnz ret_from_intr
 	decl %gs:pda_irqcount
 	leaveq
-	IPIPE_TRACE_IRQS_ON
 	CFI_DEF_CFA_REGISTER	rsp
 	CFI_ADJUST_CFA_OFFSET	-8
 	testl $3,CS-ARGOFFSET(%rsp)
Index: linux-2.6.23.1-xeno_64/arch/x86_64/kernel/ipipe.c
===================================================================
--- linux-2.6.23.1-xeno_64.orig/arch/x86_64/kernel/ipipe.c
+++ linux-2.6.23.1-xeno_64/arch/x86_64/kernel/ipipe.c
@@ -760,10 +760,6 @@ finalize:
 
 finalize_nosync:
 
-#ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-	ipipe_trace_end(irq);
-#endif /* CONFIG_IPIPE_TRACE_IRQSOFF */
-
 	if (!ipipe_root_domain_p ||
 	    test_bit(IPIPE_STALL_FLAG, &ipipe_root_cpudom_var(status)))
 		return 0;
@@ -771,18 +767,6 @@ finalize_nosync:
 	return 1;
 }
 
-#ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-void notrace __ipipe_trace_irqs_off(void)
-{
-	ipipe_trace_begin(0x80000000);
-}
-
-void notrace __ipipe_trace_irqs_on(void)
-{
-	ipipe_trace_end(0x80000000);
-}
-#endif /* CONFIG_IPIPE_TRACE_IRQSOFF */
-
 EXPORT_SYMBOL(__ipipe_tick_irq);
 EXPORT_SYMBOL(ipipe_critical_enter);
 EXPORT_SYMBOL(ipipe_critical_exit);
Index: linux-2.6.23.1-xeno_64/arch/x86_64/lib/thunk.S
===================================================================
--- linux-2.6.23.1-xeno_64.orig/arch/x86_64/lib/thunk.S
+++ linux-2.6.23.1-xeno_64/arch/x86_64/lib/thunk.S
@@ -56,11 +56,6 @@
 	thunk_retrax __ipipe_preempt_schedule_irq_thunk,__ipipe_preempt_schedule_irq
 #endif	
 	thunk_retrax __ipipe_syscall_root_thunk,__ipipe_syscall_root
-#ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-	thunk __ipipe_trace_irqs_on_thunk,__ipipe_trace_irqs_on
-	thunk __ipipe_trace_irqs_off_thunk,__ipipe_trace_irqs_off
-	thunk ipipe_trace_begin_thunk,ipipe_trace_begin
-#endif	
 #endif
 	
 	/* SAVE_ARGS below is used only for the .cfi directives it contains. */
Index: linux-2.6.23.1-xeno_64/include/asm-x86_64/irqflags.h
===================================================================
--- linux-2.6.23.1-xeno_64.orig/include/asm-x86_64/irqflags.h
+++ linux-2.6.23.1-xeno_64/include/asm-x86_64/irqflags.h
@@ -241,19 +241,12 @@ static inline void local_irq_enable_hw_n
 #endif /* CONFIG_IPIPE_TRACE_IRQSOFF */
 
 #else /* __ASSEMBLY__: */
-#ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-#define IPIPE_TRACE_IRQS_ON	call __ipipe_trace_irqs_on_thunk
-#define IPIPE_TRACE_IRQS_OFF	call __ipipe_trace_irqs_off_thunk
-#else /* !CONFIG_IPIPE_TRACE_IRQSOFF */
-#define IPIPE_TRACE_IRQS_ON
-#define IPIPE_TRACE_IRQS_OFF
-#endif /* !CONFIG_IPIPE_TRACE_IRQSOFF */
 # ifdef CONFIG_TRACE_IRQFLAGS
-#  define TRACE_IRQS_ON	IPIPE_TRACE_IRQS_ON; call trace_hardirqs_on_thunk
-#  define TRACE_IRQS_OFF	IPIPE_TRACE_IRQS_OFF; call trace_hardirqs_off_thunk
+#  define TRACE_IRQS_ON		call trace_hardirqs_on_thunk
+#  define TRACE_IRQS_OFF	call trace_hardirqs_off_thunk
 # else
-#  define TRACE_IRQS_ON	IPIPE_TRACE_IRQS_ON
-#  define TRACE_IRQS_OFF	IPIPE_TRACE_IRQS_OFF
+#  define TRACE_IRQS_ON
+#  define TRACE_IRQS_OFF
 # endif
 #endif
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Philippe Gerum]

Jan Kiszka wrote:
> Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
> over-instrumentation.

I understand the point of grouping ipipe_trace_begin/ipipe_trace_end
statements inside the .interrupt macros and using a lightweight thunk
code since we are already covered by the SAVE_ARGS prologue, but I find
the following hunk suspicious, since unlike i386, we do not virtualize
inline sti/cli ops for x86_64. My concern is that removing this
instrumentation would leave us naked in the cold the day some subtle
upstream change introduces a hw masked section we don't immediately
notice; such trace points would precisely help us spotting it.

--- linux-2.6.23.1-xeno_64.orig/include/asm-x86_64/irqflags.h
+++ linux-2.6.23.1-xeno_64/include/asm-x86_64/irqflags.h
@@ -241,19 +241,12 @@ static inline void local_irq_enable_hw_n
 #endif /* CONFIG_IPIPE_TRACE_IRQSOFF */

 #else /* __ASSEMBLY__: */
-#ifdef CONFIG_IPIPE_TRACE_IRQSOFF
-#define IPIPE_TRACE_IRQS_ON	call __ipipe_trace_irqs_on_thunk
-#define IPIPE_TRACE_IRQS_OFF	call __ipipe_trace_irqs_off_thunk
-#else /* !CONFIG_IPIPE_TRACE_IRQSOFF */
-#define IPIPE_TRACE_IRQS_ON
-#define IPIPE_TRACE_IRQS_OFF
-#endif /* !CONFIG_IPIPE_TRACE_IRQSOFF */
 # ifdef CONFIG_TRACE_IRQFLAGS
-#  define TRACE_IRQS_ON	IPIPE_TRACE_IRQS_ON; call trace_hardirqs_on_thunk
-#  define TRACE_IRQS_OFF	IPIPE_TRACE_IRQS_OFF; call
trace_hardirqs_off_thunk
+#  define TRACE_IRQS_ON		call trace_hardirqs_on_thunk
+#  define TRACE_IRQS_OFF	call trace_hardirqs_off_thunk
 # else
-#  define TRACE_IRQS_ON	IPIPE_TRACE_IRQS_ON
-#  define TRACE_IRQS_OFF	IPIPE_TRACE_IRQS_OFF
+#  define TRACE_IRQS_ON
+#  define TRACE_IRQS_OFF
 # endif
 #endif

This patch aligns x86_64 in this regard with i386
> (and hopefully the other tracer-supporting archs too). It also gets rid
> of related *_thunk indirections (I wonder if we cannot kill more of
> them...).

We could probably kill the __ipipe_preempt_schedule_irq thunk quite
easily, since we branch to exit_intr which starts with a very limited
register context. However, replacing the thunk routing
__ipipe_syscall_root would require more work, since the syscall arg
registers are expected to survive the call, and those are not
callee-saved according to the AMD64 ABI (which makes sense). We would at
least have to pull all the potentially spilled regs from the saved
exception frame upon return from the I-pipe hook. Ok, this would save a
few cycles we currently spend saving them...

-- 
Philippe.

Re: [Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Jan Kiszka]

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

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
>> over-instrumentation.
> 
> I understand the point of grouping ipipe_trace_begin/ipipe_trace_end
> statements inside the .interrupt macros and using a lightweight thunk
> code since we are already covered by the SAVE_ARGS prologue, but I find
> the following hunk suspicious, since unlike i386, we do not virtualize
> inline sti/cli ops for x86_64. My concern is that removing this
> instrumentation would leave us naked in the cold the day some subtle
> upstream change introduces a hw masked section we don't immediately
> notice; such trace points would precisely help us spotting it.

The situation is not that different compared to i386:
 - we do not add or remove enabling/disabled points
 - we rely on the correctness of mainline here, so we are not on our own
   anyway
 - if things actually change in mainline, the tracer will probably be
   the last indicator for it (patching breaks and/or leaking clis will
   fairly visible impact)

On the other side, there was a good reason not to instrument cli/sti in
the assembly exit/entry code on i386, just like it is on x86_64: The
effort to get worthwhile outputs is noticeable. As you see, the current
approach is not very helpful due to loosing the caller context (thanks
to the thunks). And those points only mark uninteresting micro paths,
thus create a lot of noise in normal traces.

So, I see not good reason for fixing this instrumentation and still vote
for killing it.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
>>> over-instrumentation.
>> I understand the point of grouping ipipe_trace_begin/ipipe_trace_end
>> statements inside the .interrupt macros and using a lightweight thunk
>> code since we are already covered by the SAVE_ARGS prologue, but I find
>> the following hunk suspicious, since unlike i386, we do not virtualize
>> inline sti/cli ops for x86_64. My concern is that removing this
>> instrumentation would leave us naked in the cold the day some subtle
>> upstream change introduces a hw masked section we don't immediately
>> notice; such trace points would precisely help us spotting it.
> 
> The situation is not that different compared to i386:
>  - we do not add or remove enabling/disabled points
>  - we rely on the correctness of mainline here, so we are not on our own
>    anyway

No we don't. Never, really. Gilles just found a bug in mainline's ARM
syscall epilogue, which would run do_notify_resume() with hw interrupts
off. Old virtualization patches had to face a subtle change in the
exception #14 handling with x86, going from a trap gate to an interrupt
gate, which basically means that hw interrupts where forced off
branching to the handler in the latter case, unlike in the former one.

>  - if things actually change in mainline, the tracer will probably be
>    the last indicator for it (patching breaks and/or leaking clis will
>    fairly visible impact)
> 

Sorry, I don't get what you mean here.  Could you explain your rationale
a bit more? The scenario I'm thinking of is when mainline adds some code
to entry.S which may call into kernel routines, adding unnoticed latency
to vanilla, but enough to be significant for any real-time activity (say
> 5us).

> On the other side, there was a good reason not to instrument cli/sti in
> the assembly exit/entry code on i386, just like it is on x86_64: The
> effort to get worthwhile outputs is noticeable.

Again, most of (pre-existing) cli/sti are virtualized by Adeos on x86,
whilst they are not on x86_64, which means that x86 did not have to
instrument the code about changes in the hw masking state anyway.

As you see, the current
> approach is not very helpful due to loosing the caller context (thanks
> to the thunks). And those points only mark uninteresting micro paths,
> thus create a lot of noise in normal traces.
> 

These are two separate issues: knowing whether the IRQs off flag is
raised, and knowing which precise code spot reported this. The former is
still a valuable information, despite the latter would be missing. At
least, this tells you where to start digging. I agree wrt the noise
issue, but this is a bit like an umbrella, it starts being useful only
when it actually rains.

Btw, the caller's context is lost because we are relying on
__builtin_return_address which imposes a well-defined call frame layout,
we could find a more ad hoc way to handle the case of assembly-written
call sites to pass this information to the tracer.

> So, I see not good reason for fixing this instrumentation and still vote
> for killing it.
> 

This is 100% tracer-related code which won't break anything, so I'm
going to apply this patch if you want this to be merged. Still, you may
also want to consider my point regarding vanilla's perceived
infallibility. As a matter of fact, vanilla has some bugs too, and aside
of that, its perception of what a pathological latency spot is may be
very different (i.e. relaxed) compared to ours in some circumstances.

> Jan
> 


-- 
Philippe.

Re: [Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Jan Kiszka]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
>>>> over-instrumentation.
>>> I understand the point of grouping ipipe_trace_begin/ipipe_trace_end
>>> statements inside the .interrupt macros and using a lightweight thunk
>>> code since we are already covered by the SAVE_ARGS prologue, but I find
>>> the following hunk suspicious, since unlike i386, we do not virtualize
>>> inline sti/cli ops for x86_64. My concern is that removing this
>>> instrumentation would leave us naked in the cold the day some subtle
>>> upstream change introduces a hw masked section we don't immediately
>>> notice; such trace points would precisely help us spotting it.
>> The situation is not that different compared to i386:
>>  - we do not add or remove enabling/disabled points
>>  - we rely on the correctness of mainline here, so we are not on our own
>>    anyway
> 
> No we don't. Never, really. Gilles just found a bug in mainline's ARM
> syscall epilogue, which would run do_notify_resume() with hw interrupts
> off. Old virtualization patches had to face a subtle change in the
> exception #14 handling with x86, going from a trap gate to an interrupt
> gate, which basically means that hw interrupts where forced off
> branching to the handler in the latter case, unlike in the former one.

For such things we are already recording the hard IRQ mask with each and
every trace point. And as long as we run with I-pipe through many common
code paths in entry.S like Linux, we _do_ rely on their correctness, or
better on us having carefully reviewed the code and ongoing mainline
changes in new kernels.

> 
>>  - if things actually change in mainline, the tracer will probably be
>>    the last indicator for it (patching breaks and/or leaking clis will
>>    fairly visible impact)
>>
> 
> Sorry, I don't get what you mean here.  Could you explain your rationale
> a bit more? The scenario I'm thinking of is when mainline adds some code
> to entry.S which may call into kernel routines, adding unnoticed latency
> to vanilla, but enough to be significant for any real-time activity (say
>> 5us).

Then these quirks will show up in those frozen paths we take with
testsuite tools. IRQS_OFF tracing will only report it by itself if 5 us
happen to be to longest path in the kernel with hard interrupts enable.
Do you expect this to happen?

> 
>> On the other side, there was a good reason not to instrument cli/sti in
>> the assembly exit/entry code on i386, just like it is on x86_64: The
>> effort to get worthwhile outputs is noticeable.
> 
> Again, most of (pre-existing) cli/sti are virtualized by Adeos on x86,
> whilst they are not on x86_64, which means that x86 did not have to
> instrument the code about changes in the hw masking state anyway.
> 
> As you see, the current
>> approach is not very helpful due to loosing the caller context (thanks
>> to the thunks). And those points only mark uninteresting micro paths,
>> thus create a lot of noise in normal traces.
>>
> 
> These are two separate issues: knowing whether the IRQs off flag is
> raised, and knowing which precise code spot reported this. The former is
> still a valuable information, despite the latter would be missing. At
> least, this tells you where to start digging. I agree wrt the noise
> issue, but this is a bit like an umbrella, it starts being useful only
> when it actually rains.
> 
> Btw, the caller's context is lost because we are relying on
> __builtin_return_address which imposes a well-defined call frame layout,
> we could find a more ad hoc way to handle the case of assembly-written
> call sites to pass this information to the tracer.
> 
>> So, I see not good reason for fixing this instrumentation and still vote
>> for killing it.
>>
> 
> This is 100% tracer-related code which won't break anything, so I'm
> going to apply this patch if you want this to be merged. Still, you may
> also want to consider my point regarding vanilla's perceived
> infallibility. As a matter of fact, vanilla has some bugs too, and aside
> of that, its perception of what a pathological latency spot is may be
> very different (i.e. relaxed) compared to ours in some circumstances.

This particular instrumentation will not tell you by itself that
something broke regarding cli/sti in [ia32]entry.S. The current version
will not even be useful if you received noticed differently that
something is fishy. You can fix the latter, but I would rather spent the
required time on reading mainline git commits to *entry.S - more
efficient IMHO because it addresses the root of (potential!) problems.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Adeos-main] [PATCH] x86_64: cleanup IRQ on/off tracing

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Philippe Gerum wrote:
>>>> Jan Kiszka wrote:
>>>>> Tracing of IRQ on/off paths on x86_64 currently suffers from heavy
>>>>> over-instrumentation.
>>>> I understand the point of grouping ipipe_trace_begin/ipipe_trace_end
>>>> statements inside the .interrupt macros and using a lightweight thunk
>>>> code since we are already covered by the SAVE_ARGS prologue, but I find
>>>> the following hunk suspicious, since unlike i386, we do not virtualize
>>>> inline sti/cli ops for x86_64. My concern is that removing this
>>>> instrumentation would leave us naked in the cold the day some subtle
>>>> upstream change introduces a hw masked section we don't immediately
>>>> notice; such trace points would precisely help us spotting it.
>>> The situation is not that different compared to i386:
>>>  - we do not add or remove enabling/disabled points
>>>  - we rely on the correctness of mainline here, so we are not on our own
>>>    anyway
>> No we don't. Never, really. Gilles just found a bug in mainline's ARM
>> syscall epilogue, which would run do_notify_resume() with hw interrupts
>> off. Old virtualization patches had to face a subtle change in the
>> exception #14 handling with x86, going from a trap gate to an interrupt
>> gate, which basically means that hw interrupts where forced off
>> branching to the handler in the latter case, unlike in the former one.
> 
> For such things we are already recording the hard IRQ mask with each and
> every trace point. And as long as we run with I-pipe through many common
> code paths in entry.S like Linux, we _do_ rely on their correctness, or
> better on us having carefully reviewed the code and ongoing mainline
> changes in new kernels.

My point is that we might not have enough "density" in our
instrumentation anymore to easily spot such regressions. Again, it's not
like I'm saying that using the tracer is the only method to chase
latencies, but since we have it now, we may want to use it whenever
possible. I still don't agree on the fact that we rely on Linux's
correctness though, that's partly why we indeed do analyze the standard
code paths as you mentioned, actually.

> 
>>>  - if things actually change in mainline, the tracer will probably be
>>>    the last indicator for it (patching breaks and/or leaking clis will
>>>    fairly visible impact)
>>>
>> Sorry, I don't get what you mean here.  Could you explain your rationale
>> a bit more? The scenario I'm thinking of is when mainline adds some code
>> to entry.S which may call into kernel routines, adding unnoticed latency
>> to vanilla, but enough to be significant for any real-time activity (say
>>> 5us).
> 
> Then these quirks will show up in those frozen paths we take with
> testsuite tools. IRQS_OFF tracing will only report it by itself if 5 us
> happen to be to longest path in the kernel with hard interrupts enable.
> Do you expect this to happen?

The point is that you may prefer having to analyze a code path made of
30 IRQs off spots, rather than one made of 200 or more. Ok, let's
illustrate the scenario I have in mind:

Here is what would be the problematic kind of hunk, mainline might
decide to add:

	cli
	TRACE_IRQS_OFF
	...
/* We get there hw interrupts off */
sysret_careful:
	bt $TIF_NEED_RESCHED,%edx
-	jnc sysret_signal
+	jnc sysret_whatever
	TRACE_IRQS_ON
	sti
	pushq %rdi
	CFI_ADJUST_CFA_OFFSET 8
	call schedule
	popq  %rdi
	CFI_ADJUST_CFA_OFFSET -8
	jmp sysret_check

	/* Handle a signal */
sysret_signal:
	TRACE_IRQS_ON
	sti
	...

+ sysret_whatever:
	leaq -ARGOFFSET(%rsp),%rdi
	call whatever_this_does_is_expensive
	jmp sysret_signal

My fear is that, since we do not have instrumentation in TRACE_IRQ_xx
macros anymore, we may have lesser information to spot the call to
whatever_this_does_is_expensive() as being the potential latency raiser.

I do agree that we might catch this by dissecting each and every hunk
mainline adds to entry.S, but still, we lose some runtime debugging support.

> 
>>> On the other side, there was a good reason not to instrument cli/sti in
>>> the assembly exit/entry code on i386, just like it is on x86_64: The
>>> effort to get worthwhile outputs is noticeable.
>> Again, most of (pre-existing) cli/sti are virtualized by Adeos on x86,
>> whilst they are not on x86_64, which means that x86 did not have to
>> instrument the code about changes in the hw masking state anyway.
>>
>> As you see, the current
>>> approach is not very helpful due to loosing the caller context (thanks
>>> to the thunks). And those points only mark uninteresting micro paths,
>>> thus create a lot of noise in normal traces.
>>>
>> These are two separate issues: knowing whether the IRQs off flag is
>> raised, and knowing which precise code spot reported this. The former is
>> still a valuable information, despite the latter would be missing. At
>> least, this tells you where to start digging. I agree wrt the noise
>> issue, but this is a bit like an umbrella, it starts being useful only
>> when it actually rains.
>>
>> Btw, the caller's context is lost because we are relying on
>> __builtin_return_address which imposes a well-defined call frame layout,
>> we could find a more ad hoc way to handle the case of assembly-written
>> call sites to pass this information to the tracer.
>>
>>> So, I see not good reason for fixing this instrumentation and still vote
>>> for killing it.
>>>
>> This is 100% tracer-related code which won't break anything, so I'm
>> going to apply this patch if you want this to be merged. Still, you may
>> also want to consider my point regarding vanilla's perceived
>> infallibility. As a matter of fact, vanilla has some bugs too, and aside
>> of that, its perception of what a pathological latency spot is may be
>> very different (i.e. relaxed) compared to ours in some circumstances.
> 
> This particular instrumentation will not tell you by itself that
> something broke regarding cli/sti in [ia32]entry.S. The current version
> will not even be useful if you received noticed differently that
> something is fishy. You can fix the latter, but I would rather spent the
> required time on reading mainline git commits to *entry.S - more
> efficient IMHO because it addresses the root of (potential!) problems.
>

It's all a matter of method, and your argument is certainly right in a
number of cases. But, we provide mechanisms to help debugging, we should
not impose policies on debugging.

> Jan
> 


-- 
Philippe.