Thread (10 messages) flat view 10 messages, 3 authors, 2018-03-09

Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from

From: Naveen N. Rao <hidden>
Date: 2018-03-09 08:15:55

Michael Ellerman wrote:
"Naveen N. Rao" [off-list ref] writes:
=20
quoted
We can't take a trap in most parts of real mode code. Instead of adding
the 'notrace' annotation to all C functions that can be invoked from
real mode, detect that we are in real mode on ftrace entry and return
back.

Signed-off-by: Naveen N. Rao <redacted>
---
This RFC only handles -mprofile-kernel to demonstrate the approach being=
=20
quoted
considered. We will need to handle other ftrace entry if we decide to=20
continue down this path.
=20
Paul and I were talking about having a paca flag for this, ie.
paca->safe_to_ftrace (or whatever). I'm not sure if you've talked to
him and decided this is a better approach.
=20
I guess I'm 50/50 on which is better, they both have pluses and minuses.
Thanks, I hadn't spoken to Paul, but I now think that this is probably=20
the better approach to take.

My earlier assumption was that we have other scenarios when we are in=20
realmode (specifically with MSR_RI unset) where we won't be able to=20
recover from a trap, during function tracing (*). I did a set of=20
experiments yesterday to verify that, but I was not able to uncover any=20
such scenarios with my brief testing. So, we seem to be functioning just=20
fine while tracing realmode C code, except for KVM.

As such, rather than blacklisting all realmode code, I think it is=20
better to be selective and just disable the tracer for KVM since we know=20
we can't take a trap there. We will be able to use the same approach if=20
we uncover additional scenarios where we can't use function tracing. I=20
will look at implementing a paca field for this purpose.

I also noticed that even with an unexpected timebase, we still seem to=20
recover just fine with a simple change:
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2629,8 +2629,8 @@ static noinline void
 rb_handle_timestamp(struct ring_buffer_per_cpu *cpu_buffer,
                    struct rb_event_info *info)
 {
-       WARN_ONCE(info->delta > (1ULL << 59),
-                 KERN_WARNING "Delta way too big! %llu ts=3D%llu write sta=
mp =3D %llu\n%s",
+       if (info->delta > (1ULL << 59))
+               pr_warn_once("Delta way too big! %llu ts=3D%llu write stamp=
 =3D %llu\n%s",
                  (unsigned long long)info->delta,
                  (unsigned long long)info->ts,
                  (unsigned long long)cpu_buffer->write_stamp,


This allowed the virtual machine to boot and we were able to trace the=20
rest of KVM C code. I only just did a boot test, so I'm not sure if=20
there are other scenarios where things can go wrong.

Steve,
Would you be willing to accept a patch like the above? Since we seem to=20
handle the larger delta just fine, I think the above change should be=20
fine?

I will still work on excluding KVM C code from being traced, but the=20
advantage with the above patch is that we will be able to trace KVM C=20
code with a small change if necessary.


- Naveen

---
(*) putting on my kprobe hat

=
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help