On 01.10.2020 14:13, Miroslav Benes wrote:
On Wed, 30 Sep 2020, Evgenii Shatokhin wrote:
Hi,
I wonder, can livepatch from the current mainline kernel patch the main
functions of kthreads, which are running or sleeping constantly? Are there any
best practices here?
No. It is a "known" limitation, "" because we discussed it a couple of
times (at least with Petr), but it is not documented :(
I wonder if it is really an issue practically. I haven't met a case
yet when we wanted to patch such thing. But yes, you're correct, it is not
possible.
Well, I have recently encountered such case, with kaio_fsync_thread()
function from our custom kernel, the code at the URL below. Our
customers were interested in a particular bug fix there: there was a
race, potentially leading to data corruption.
We still use the old-style kpatch.ko-based patches for that kernel
version, so we definitely cannot deliver the fix via a live kernel
patch, only a regular kernel update will do. That made me wonder if the
modern livepatch could handle it.
I mean, suppose we have a function which runs in a kthread (passed to
kthread_create()) and is organized like this:
while (!kthread_should_stop()) {
...
DEFINE_WAIT(_wait);
for (;;) {
prepare_to_wait(waitq, &_wait, TASK_INTERRUPTIBLE);
if (we_have_requests_to_process || kthread_should_stop())
break;
schedule();
}
finish_wait(waitq, &_wait);
...
if (we_have_requests_to_process)
process_one_request();
...
}
(The question appeared when I was looking at the following code:
https://src.openvz.org/projects/OVZ/repos/vzkernel/browse/drivers/block/ploop/io_kaio.c?at=refs%2Ftags%2Frh7-3.10.0-1127.8.2.vz7.151.14#478)
The kthread is always running and never exits the kernel.
I could rewrite the function to add klp_update_patch_state() somewhere, but
would it help?
In fact, we used exactly this approach in, now obsolete, kGraft. All
kthreads had to be manually annotated somewhere safe, where safe meant
that the thread could be switched to a new universe without the problem
wrt to calling old/new functions in the loop...
No locks are held right before and after "schedule()", and the thread is not
processing any requests at that point.
... like this.
But even if I place
klp_update_patch_state(), say, just before schedule(), it would just switch
task->patch_state for that kthread.
Correct.
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().
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.
Looks like we can only switch to the patched code of the function at the
beginning, via Ftrace hook. So, if the function is constantly running or
sleeping, it seems, it cannot be live-patched.
Yes and no. Normally, a task cannot run indefinitely and if it sleeps, we
can deal with that (thanks to stack checking or signaling/kicking the
task), but kthreads' main loops are special.
Is that so? Are there any workarounds?
Petr, do you remember the crazy workarounds we talked about? My head is
empty now. And I am sure, Nicolai could come up with something.
Interesting. I am all ears.
Thanks!
Evgenii
Thanks
Miroslav
.
