Re: Patching kthread functions

[Evgenii Shatokhin <eshatokhin@virtuozzo.com>]

On 01.10.2020 17:46, Petr Mladek wrote:
> On Thu 2020-10-01 13:13:07, 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.
>>   
>>> 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();
>>>    ...
>>> }
> 
> Crazy hack would be to patch only process_one_request() the following way:
> 
> 1. Put the fixed main loop into the new process_one_request() function.
> 
> 2. Put the original process_one_request() code into another function,
>     e.g. do_process_one_request_for_real() and call it from the
>     fixed loop.
> 
> Does it make any sense or should I provide some code?

I think, I get the idea, thanks!

So, the thread would loop in the new process_one_request() and, with 
necessary care taken, would exit both loops correctly when needed.

In our case (kaio_fsync_thread() at the mentioned URL) 
process_one_request() is actually not a separate function but rather 
part of the same thread function. So, in this particular case it would 
not help. If we refactor the function in some future versions, this 
could be an option.

But, yes, such patch would need to remain applied forever, no updates 
and no unloads, which is a downside.

> 
> Be aware that such patch could not get reverted because it would never
> leave the new main loop.
> 
> Best Regards,
> Petr
> .
> 

Regards,
Evgenii