Thread (9 messages) 9 messages, 3 authors, 2020-10-30

Re: [PATCH 6/9] livepatch/ftrace: Add recursion protection to the ftrace callback

From: Miroslav Benes <mbenes@suse.cz>
Date: 2020-10-30 09:49:02
Also in: lkml

quoted
quoted
quoted
+	bit = ftrace_test_recursion_trylock();
+	if (bit < 0)
+		return;  
This means that the original function will be called in case of recursion. 
That's probably fair, but I'm wondering if we should at least WARN about 
it.  
Yeah, the early return might break the consistency model and
unexpected things might happen. We should be aware of it.
Please use:

	if (WARN_ON_ONCE(bit < 0))
		return;

WARN_ON_ONCE() might be part of the recursion. But it should happen
only once. IMHO, it is worth the risk.

Otherwise it looks good.
Perhaps we can add that as a separate patch, because this patch doesn't add
any real functionality change. It only moves the recursion testing from the
helper function (which ftrace wraps all callbacks that do not have the
RECURSION flags set, including this one) down to your callback.

In keeping with one patch to do one thing principle, the added of
WARN_ON_ONCE() should be a separate patch, as that will change the
functionality.

If that WARN_ON_ONCE() breaks things, I'd like it to be bisected to another
patch other than this one.
Works for me.

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