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

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>=20
>> 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 <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> This RFC only handles -mprofile-kernel to demonstrate the approach being=
=20
>> 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

=