Re: [PATCH] time/sched_clock: mark sched_clock_read_begin as notrace

[Steven Rostedt]

On Tue, 29 Sep 2020 09:13:33 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Mon, Sep 28, 2020 at 05:33:31PM -0400, Steven Rostedt wrote:
> > On Mon, 28 Sep 2020 12:58:59 +0200
> > Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >   
> > > > -struct clock_read_data *sched_clock_read_begin(unsigned int *seq)
> > > > +notrace struct clock_read_data *sched_clock_read_begin(unsigned int *seq)
> > > >  {
> > > >  	*seq = raw_read_seqcount_latch(&cd.seq);
> > > >  	return cd.read_data + (*seq & 1);    
> > > 
> > > At the very least sched_clock_read_retry() should also be marked such.
> > > 
> > > But Steve, how come x86 works? Our sched_clock() doesn't have notrace on
> > > at all.  
> > 
> > It's because of that magic in the Makefile that you love so much ;-)
> > 
> > CFLAGS_REMOVE_tsc.o = -pg  
> 
> ARGH!!, I really should write a script to fix up that mess :/

Please don't.

I did this because these files contain lots of functions, that if traced
can cause function tracing to reboot without any information, and as you
have recently found, they are very hard to find when they do happen.

I much rather blacklist an entire file, than to spend time debugging what
function caused the system to reboot.

I'd be OK with both. That is, add "notrace" to all the functions in the
file that isn't traced, just for documentation purposes. Perhaps call it
something else:

  file_notrace  ?

and have a comment about it being "this is just to let you know that the
functions are not traced because of the file".

And this "file_notrace" could even be:

/*
 * Denote functions that are not traced because the make file removes the
 * tracing features via a CONFIG_REMOVE_xxx.o = CC_FLAGS_FTRACE or similar.
 * This is for documentation purposes only, to remind people why a function
 * is not being traced.
 */
#define file_notrace /* Not traced because the file is blacklisted */

We could also add:

#define dir_notrace /* Not traced because the entire directory is blacklisted */

-- Steve