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
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