Re: mmotm 2022-02-11-15-07 uploaded (objtool: ftrace_likely_update)

[Steven Rostedt]

On Sat, 12 Feb 2022 09:06:49 +0100
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> Yes, TRACE_BRANCH_PROFILING and PROFILE_ALL_BRANCHES are fundamentally
> broken and I have no intention of trying to fix them.
> 
> The moment we pull PTI into noinstr C code this will result in insta
> boot fail.

Actually, I don't think anyone has every used the "tracers" for this, and I
will be happy to get rid of it:

void ftrace_likely_update(struct ftrace_likely_data *f, int val,
			  int expect, int is_constant)
{
	unsigned long flags = user_access_save();

	/* A constant is always correct */
	if (is_constant) {
		f->constant++;
		val = expect;
	}


------8<------
	/*
	 * I would love to have a trace point here instead, but the
	 * trace point code is so inundated with unlikely and likely
	 * conditions that the recursive nightmare that exists is too
	 * much to try to get working. At least for now.
	 */
	trace_likely_condition(f, val, expect);
----->8-------

	/* FIXME: Make this atomic! */
	if (val == expect)
		f->data.correct++;
	else
		f->data.incorrect++;

	user_access_restore(flags);
}
EXPORT_SYMBOL(ftrace_likely_update);

The above with the cut lines I added.

I still use the likely and unlikely counters. Would it be possible to mark
that function as "noinstr" and still record them (I don't care if there's
races where we miss a few or add a few too many). But they have been really
affective in finding bad locations of likely and unlikely callers.

As I said. I have no problem with removing the trace portion of that code.
It was more of an academic exercise than a useful one, but the counters
are still very useful to have.

-- Steve