-mprofile-kernel vs. notrace in ppc64(le) Linux kernels

As I mentioned earlier this year, it's a bad idea to call _mcount from
MMU helper functions (e.g. hash_page...), when the profiling/tracing/
live-patching/whatever framewok might in turn cause another such fault.
Jikos suggested to use fine-grained control of these functions with the
"notrace" keyword in the Linux kernel. It is mapped to GCC's (4.8, FWIW)
__attribute__((no_instrument_function)), which, to my surprise, works
for -p and -pg nicely, but does not affect -mprofile-kernel at all!

Should we consider this a bug? IMHO it is. I can see in the GCC sources
that -mprofile-kernel is more like a low-level hack in rs6000.c,
quite far below the RTL code generator, so the no_instrument_function
attribute is probably hard to check for.

What is -mprofile-kernel good for, if it bears such a risk of crashing
the kernel? Is it the right hook for ppc64 live patching? How to protect
those critical functions? Filter -mprofile-kernel for those object files?
Ask the GCC experts to fix this?

Any ideas welcome!

	Torsten

On Sat, Sep 26, 2015 at 04:30:08PM +0200, Torsten Duwe wrote:

As I mentioned earlier this year, it's a bad idea to call _mcount from
MMU helper functions (e.g. hash_page...), when the profiling/tracing/
live-patching/whatever framewok might in turn cause another such fault.
Jikos suggested to use fine-grained control of these functions with the
"notrace" keyword in the Linux kernel. It is mapped to GCC's (4.8, FWIW)
__attribute__((no_instrument_function)), which, to my surprise, works
for -p and -pg nicely, but does not affect -mprofile-kernel at all!

That is surprising; please file a GCC bug.

Should we consider this a bug? IMHO it is. I can see in the GCC sources
that -mprofile-kernel is more like a low-level hack in rs6000.c,
quite far below the RTL code generator, so the no_instrument_function
attribute is probably hard to check for.

What is -mprofile-kernel good for, if it bears such a risk of crashing
the kernel?

It didn't work when combined with other fringe functionality.  It happens.

Is it the right hook for ppc64 live patching? How to protect
those critical functions? Filter -mprofile-kernel for those object files?

That might work.

Ask the GCC experts to fix this?

Yes please.  You probably need a workaround for released compilers though.


Segher

Hi,

On Sat, Sep 26, 2015 at 04:30:08PM +0200, Torsten Duwe wrote:

quoted

As I mentioned earlier this year, it's a bad idea to call _mcount
from MMU helper functions (e.g. hash_page...), when the
profiling/tracing/ live-patching/whatever framewok might in turn
cause another such fault. Jikos suggested to use fine-grained
control of these functions with the "notrace" keyword in the Linux
kernel. It is mapped to GCC's (4.8, FWIW)
__attribute__((no_instrument_function)), which, to my surprise,
works for -p and -pg nicely, but does not affect -mprofile-kernel
at all!

That is surprising; please file a GCC bug.

This sounds like the issue below, fixed by Pat.

Anton
--

commit ccebc2aad32917f3371ae6d5774f689c86ace0b2
Author: pthaugen <pthaugen@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Jan 15 23:48:12 2014 +0000

    	* config/rs6000/rs6000.c (rs6000_output_function_prologue): Check if
    	current procedure should be profiled.
    
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@206650 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2781bd0..0788036 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog

@@ -1,3 +1,8 @@
+2014-01-15  Pat Haugen  <pthaugen@us.ibm.com>
+
+	* config/rs6000/rs6000.c (rs6000_output_function_prologue): Check if
+	current procedure should be profiled.
+
 2014-01-15  Andrew Pinski  <apinski@cavium.com>
 
 	* config/aarch64/aarch64.c (aarch64_register_move_cost): Correct cost

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 289b52c..a479219 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c

@@ -23198,7 +23198,7 @@ rs6000_output_function_prologue (FILE *file,
   /* Output -mprofile-kernel code.  This needs to be done here instead of
      in output_function_profile since it must go after the ELFv2 ABI
      local entry point.  */
-  if (TARGET_PROFILE_KERNEL)
+  if (TARGET_PROFILE_KERNEL && crtl->profile)
     {
       gcc_assert (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2);
       gcc_assert (!TARGET_32BIT);

On Sun, Sep 27, 2015 at 09:03:48AM +1000, Anton Blanchard wrote:

Yes, indeed!
I can see the fix in the 4.9 branch, it's in 5.x and 6.x, but _not_ in 4.8.5 :(
That's what you get when you use "stable" distributions.

Thanks a lot for the pointer!

	Torsten

Torsten Duwe [off-list ref] writes:

As I mentioned earlier this year, it's a bad idea to call _mcount from
MMU helper functions (e.g. hash_page...), when the profiling/tracing/
live-patching/whatever framewok might in turn cause another such fault.
Jikos suggested to use fine-grained control of these functions with the
"notrace" keyword in the Linux kernel. It is mapped to GCC's (4.8, FWIW)
__attribute__((no_instrument_function)), which, to my surprise, works
for -p and -pg nicely, but does not affect -mprofile-kernel at all!

IIRC from when I was working on GCOV support in OPAL firmware (skiboot),
this doesn't affect the code generated by -fprofile-arcs, so may not
completely do what you want.