On Fri, 2 Oct 2020, Evgenii Shatokhin wrote:

On 02.10.2020 14:53, Miroslav Benes wrote:

quoted

quoted

quoted

quoted

The old function will continue running, right?

Correct. It will, however, call new functions.

Ah, I see.

So, I guess, our best bet would be to rewrite the thread function so that
it
contains just the event loop and calls other non-inline functions to
actually
process the requests. And, perhaps, - place klp_update_patch_state() before
schedule().

Yes, that might be the way. klp_update_patch_state() might not be even
needed. If the callees are live patched, the kthread would be migrated
thanks to stack checking once a task leaves the callee.

You mean, the task runs the callee, then goes to schedule(), then, while it
waits, livepatch core checks its stack, sees no target functions there and
switches patch_state?

Yes. Once the task gets out of the target function (or the set of 
functions), its patch state can be changed. If it sleeps (interruptedly) 
in the target function, we wake it up so it can get out 
(klp_send_signals()).
 

quoted

quoted

This will not help with this particular kernel version but could make it
possible to live-patch the request-processing functions in the future
kernel
versions. The main thread function will remain unpatchable but it will call
the patched functions once we switch the patch_state for the thread.

Yes. The only issue is if the intended fix changes the semantics which is
incompatible between old and new functions (livepatch consistency model is
LEAVE_PATCHED_SET, SWITCH_THREAD, see
https://lore.kernel.org/lkml/20141107140458.GA21774@suse.cz/ (local) for the
explanation if interested).

Yes, I have read that.

In our case, the fix only adds a kind of lock/unlock around the part of the
function processing actual requests. The implementation is more complex, but,
essentially, it is lock + unlock. The code not touched by the patch already
handles such locking OK, so it can work both with old and the new versions of
patched functions. And - even if some threads use the old functions and some -
the new ones. So, I guess, it should be fine.

Ok, that should be fine.

Miroslav