RE: livepatch: change to a per-task consistency model
On Tue, 24 May 2016, David Laight wrote: > > > Related, please can we have a flag for the sleep and/or process so that > > > an uninterruptible sleep doesn't trigger the 'hung task' detector > > > > TASK_KILLABLE > > Not sure that does what I want. > It appears to allow some 'kill' actions to wake the process. > I'm sure I've looked at the 'hung task' code since 2007. The trick is the if (t->state == TASK_UNINTERRUPTIBLE) test in check_hung_uninterruptible_tasks(). That makes sure that TASK_KILLABLE tasks (e.g. waiting on NFS I/O, but not limited only to that; feel free to set it whereever you need) are skipped. Please note that TASK_KILLABLE is actually a _mask_ that includes TASK_UNINTERRUPTIBLE as well; therefore the '==' test skips such tasks. -- Jiri Kosina SUSE Labs
RE: livepatch: change to a per-task consistency model
On Tue, 24 May 2016, David Laight wrote: > > > Related, please can we have a flag for the sleep and/or process so that > > > an uninterruptible sleep doesn't trigger the 'hung task' detector > > > > TASK_KILLABLE > > Not sure that does what I want. > It appears to allow some 'kill' actions to wake the process. > I'm sure I've looked at the 'hung task' code since 2007. The trick is the if (t->state == TASK_UNINTERRUPTIBLE) test in check_hung_uninterruptible_tasks(). That makes sure that TASK_KILLABLE tasks (e.g. waiting on NFS I/O, but not limited only to that; feel free to set it whereever you need) are skipped. Please note that TASK_KILLABLE is actually a _mask_ that includes TASK_UNINTERRUPTIBLE as well; therefore the '==' test skips such tasks. -- Jiri Kosina SUSE Labs
RE: livepatch: change to a per-task consistency model
From: Jiri Kosina > Sent: 23 May 2016 19:45 > > Related, please can we have a flag for the sleep and/or process so that > > an uninterruptible sleep doesn't trigger the 'hung task' detector > > TASK_KILLABLE Not sure that does what I want. It appears to allow some 'kill' actions to wake the process. I'm sure I've looked at the 'hung task' code since 2007. > > and also stops the process counting towards the 'load average'. > > TASK_NOLOAD Ah, that was added in May 2015. Not surprising I didn't know about it. I'll leave the code doing: set_current_state(signal_pending(current) ? TASK_UNINTERRUPTIBLE ? TASK_INTERRUPTIBLE); for a while longer. David
RE: livepatch: change to a per-task consistency model
From: Jiri Kosina > Sent: 23 May 2016 19:45 > > Related, please can we have a flag for the sleep and/or process so that > > an uninterruptible sleep doesn't trigger the 'hung task' detector > > TASK_KILLABLE Not sure that does what I want. It appears to allow some 'kill' actions to wake the process. I'm sure I've looked at the 'hung task' code since 2007. > > and also stops the process counting towards the 'load average'. > > TASK_NOLOAD Ah, that was added in May 2015. Not surprising I didn't know about it. I'll leave the code doing: set_current_state(signal_pending(current) ? TASK_UNINTERRUPTIBLE ? TASK_INTERRUPTIBLE); for a while longer. David
RE: livepatch: change to a per-task consistency model
On Mon, 23 May 2016, David Laight wrote: > Related, please can we have a flag for the sleep and/or process so that > an uninterruptible sleep doesn't trigger the 'hung task' detector TASK_KILLABLE > and also stops the process counting towards the 'load average'. TASK_NOLOAD -- Jiri Kosina SUSE Labs
RE: livepatch: change to a per-task consistency model
On Mon, 23 May 2016, David Laight wrote: > Related, please can we have a flag for the sleep and/or process so that > an uninterruptible sleep doesn't trigger the 'hung task' detector TASK_KILLABLE > and also stops the process counting towards the 'load average'. TASK_NOLOAD -- Jiri Kosina SUSE Labs
RE: livepatch: change to a per-task consistency model
From: Jiri Kosina > Sent: 18 May 2016 21:23 > On Wed, 18 May 2016, Josh Poimboeuf wrote: > > > Yeah, I think this situation -- a task sleeping on an affected function > > in uninterruptible state for a long period of time -- would be > > exceedingly rare and not something we need to worry about for now. > > Plus in case task'd be in TASK_UNINTERRUPTIBLE for more than 120s, hung > task detector would trigger anyway. Related, please can we have a flag for the sleep and/or process so that an uninterruptible sleep doesn't trigger the 'hung task' detector and also stops the process counting towards the 'load average'. In particular some kernel threads are not signalable, and do not want to be woken by signals (they exit on a specific request). David
RE: livepatch: change to a per-task consistency model
From: Jiri Kosina > Sent: 18 May 2016 21:23 > On Wed, 18 May 2016, Josh Poimboeuf wrote: > > > Yeah, I think this situation -- a task sleeping on an affected function > > in uninterruptible state for a long period of time -- would be > > exceedingly rare and not something we need to worry about for now. > > Plus in case task'd be in TASK_UNINTERRUPTIBLE for more than 120s, hung > task detector would trigger anyway. Related, please can we have a flag for the sleep and/or process so that an uninterruptible sleep doesn't trigger the 'hung task' detector and also stops the process counting towards the 'load average'. In particular some kernel threads are not signalable, and do not want to be woken by signals (they exit on a specific request). David
Re: livepatch: change to a per-task consistency model
On Wed, 18 May 2016, Josh Poimboeuf wrote: > Yeah, I think this situation -- a task sleeping on an affected function > in uninterruptible state for a long period of time -- would be > exceedingly rare and not something we need to worry about for now. Plus in case task'd be in TASK_UNINTERRUPTIBLE for more than 120s, hung task detector would trigger anyway. -- Jiri Kosina SUSE Labs
Re: livepatch: change to a per-task consistency model
On Wed, 18 May 2016, Josh Poimboeuf wrote: > Yeah, I think this situation -- a task sleeping on an affected function > in uninterruptible state for a long period of time -- would be > exceedingly rare and not something we need to worry about for now. Plus in case task'd be in TASK_UNINTERRUPTIBLE for more than 120s, hung task detector would trigger anyway. -- Jiri Kosina SUSE Labs
Re: livepatch: change to a per-task consistency model
On Wed, May 18, 2016 at 10:16:22AM +0200, Jiri Kosina wrote: > On Tue, 17 May 2016, Jessica Yu wrote: > > > What about tasks sleeping on affected functions in uninterruptible sleep > > (possibly indefinitely)? Since all signals are ignored, we wouldn't be > > able to patch those tasks in this way, right? Would that be an > > unsupported case? > > I don't think there is any better way out of this situation than > documenting that the convergence of patching could in such cases could > take quite a lot of time (well, we can pro-actively try to detect this > situation before the patching actually start, and warn about the possible > consequences). > > But let's face it, this should be pretty uncommon, because (a) it's not > realistic for the wait times to be really indefinite (b) the task is > likely to be in TASK_KILLABLE rather than just plain TASK_UNINTERRUPTIBLE. Yeah, I think this situation -- a task sleeping on an affected function in uninterruptible state for a long period of time -- would be exceedingly rare and not something we need to worry about for now. -- Josh
Re: livepatch: change to a per-task consistency model
On Wed, May 18, 2016 at 10:16:22AM +0200, Jiri Kosina wrote: > On Tue, 17 May 2016, Jessica Yu wrote: > > > What about tasks sleeping on affected functions in uninterruptible sleep > > (possibly indefinitely)? Since all signals are ignored, we wouldn't be > > able to patch those tasks in this way, right? Would that be an > > unsupported case? > > I don't think there is any better way out of this situation than > documenting that the convergence of patching could in such cases could > take quite a lot of time (well, we can pro-actively try to detect this > situation before the patching actually start, and warn about the possible > consequences). > > But let's face it, this should be pretty uncommon, because (a) it's not > realistic for the wait times to be really indefinite (b) the task is > likely to be in TASK_KILLABLE rather than just plain TASK_UNINTERRUPTIBLE. Yeah, I think this situation -- a task sleeping on an affected function in uninterruptible state for a long period of time -- would be exceedingly rare and not something we need to worry about for now. -- Josh
Re: livepatch: change to a per-task consistency model
On Tue, 17 May 2016, Jessica Yu wrote: > What about tasks sleeping on affected functions in uninterruptible sleep > (possibly indefinitely)? Since all signals are ignored, we wouldn't be > able to patch those tasks in this way, right? Would that be an > unsupported case? I don't think there is any better way out of this situation than documenting that the convergence of patching could in such cases could take quite a lot of time (well, we can pro-actively try to detect this situation before the patching actually start, and warn about the possible consequences). But let's face it, this should be pretty uncommon, because (a) it's not realistic for the wait times to be really indefinite (b) the task is likely to be in TASK_KILLABLE rather than just plain TASK_UNINTERRUPTIBLE. -- Jiri Kosina SUSE Labs
Re: livepatch: change to a per-task consistency model
On Tue, 17 May 2016, Jessica Yu wrote: > What about tasks sleeping on affected functions in uninterruptible sleep > (possibly indefinitely)? Since all signals are ignored, we wouldn't be > able to patch those tasks in this way, right? Would that be an > unsupported case? I don't think there is any better way out of this situation than documenting that the convergence of patching could in such cases could take quite a lot of time (well, we can pro-actively try to detect this situation before the patching actually start, and warn about the possible consequences). But let's face it, this should be pretty uncommon, because (a) it's not realistic for the wait times to be really indefinite (b) the task is likely to be in TASK_KILLABLE rather than just plain TASK_UNINTERRUPTIBLE. -- Jiri Kosina SUSE Labs
Re: livepatch: change to a per-task consistency model
+++ Josh Poimboeuf [28/04/16 15:44 -0500]: [snip] diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt index 6c43f6e..bee86d0 100644 --- a/Documentation/livepatch/livepatch.txt +++ b/Documentation/livepatch/livepatch.txt @@ -72,7 +72,8 @@ example, they add a NULL pointer or a boundary check, fix a race by adding a missing memory barrier, or add some locking around a critical section. Most of these changes are self contained and the function presents itself the same way to the rest of the system. In this case, the functions might -be updated independently one by one. +be updated independently one by one. (This can be done by setting the +'immediate' flag in the klp_patch struct.) But there are more complex fixes. For example, a patch might change ordering of locking in multiple functions at the same time. Or a patch @@ -86,20 +87,103 @@ or no data are stored in the modified structures at the moment. The theory about how to apply functions a safe way is rather complex. The aim is to define a so-called consistency model. It attempts to define conditions when the new implementation could be used so that the system -stays consistent. The theory is not yet finished. See the discussion at -http://thread.gmane.org/gmane.linux.kernel/1823033/focus=1828189 - -The current consistency model is very simple. It guarantees that either -the old or the new function is called. But various functions get redirected -one by one without any synchronization. - -In other words, the current implementation _never_ modifies the behavior -in the middle of the call. It is because it does _not_ rewrite the entire -function in the memory. Instead, the function gets redirected at the -very beginning. But this redirection is used immediately even when -some other functions from the same patch have not been redirected yet. - -See also the section "Limitations" below. +stays consistent. + +Livepatch has a consistency model which is a hybrid of kGraft and +kpatch: it uses kGraft's per-task consistency and syscall barrier +switching combined with kpatch's stack trace switching. There are also +a number of fallback options which make it quite flexible. + +Patches are applied on a per-task basis, when the task is deemed safe to +switch over. When a patch is enabled, livepatch enters into a +transition state where tasks are converging to the patched state. +Usually this transition state can complete in a few seconds. The same +sequence occurs when a patch is disabled, except the tasks converge from +the patched state to the unpatched state. + +An interrupt handler inherits the patched state of the task it +interrupts. The same is true for forked tasks: the child inherits the +patched state of the parent. + +Livepatch uses several complementary approaches to determine when it's +safe to patch tasks: + +1. The first and most effective approach is stack checking of sleeping + tasks. If no affected functions are on the stack of a given task, + the task is patched. In most cases this will patch most or all of + the tasks on the first try. Otherwise it'll keep trying + periodically. This option is only available if the architecture has + reliable stacks (CONFIG_RELIABLE_STACKTRACE and + CONFIG_STACK_VALIDATION). + +2. The second approach, if needed, is kernel exit switching. A + task is switched when it returns to user space from a system call, a + user space IRQ, or a signal. It's useful in the following cases: + + a) Patching I/O-bound user tasks which are sleeping on an affected + function. In this case you have to send SIGSTOP and SIGCONT to + force it to exit the kernel and be patched. See below - + b) Patching CPU-bound user tasks. If the task is highly CPU-bound + then it will get patched the next time it gets interrupted by an + IRQ. + c) Applying patches for architectures which don't yet have + CONFIG_RELIABLE_STACKTRACE. In this case you'll have to signal + most of the tasks on the system. However this isn't a complete + solution, because there's currently no way to patch kthreads + without CONFIG_RELIABLE_STACKTRACE. + + Note: since idle "swapper" tasks don't ever exit the kernel, they + instead have a kpatch_patch_task() call in the idle loop which allows + them to patched before the CPU enters the idle state. + +3. A third approach (not yet implemented) is planned for the case where + a kthread is sleeping on an affected function. In that case we could + kick the kthread with a signal and then try to patch the task from + the to-be-patched function's livepatch ftrace handler when it + re-enters the function. This will require + CONFIG_RELIABLE_STACKTRACE. + +All the above approaches may be skipped by setting the 'immediate' flag +in the 'klp_patch' struct, which will patch all tasks immediately. This +can be useful if the patch doesn't change any function or data +semantics. Note that, even
Re: livepatch: change to a per-task consistency model
+++ Josh Poimboeuf [28/04/16 15:44 -0500]: [snip] diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt index 6c43f6e..bee86d0 100644 --- a/Documentation/livepatch/livepatch.txt +++ b/Documentation/livepatch/livepatch.txt @@ -72,7 +72,8 @@ example, they add a NULL pointer or a boundary check, fix a race by adding a missing memory barrier, or add some locking around a critical section. Most of these changes are self contained and the function presents itself the same way to the rest of the system. In this case, the functions might -be updated independently one by one. +be updated independently one by one. (This can be done by setting the +'immediate' flag in the klp_patch struct.) But there are more complex fixes. For example, a patch might change ordering of locking in multiple functions at the same time. Or a patch @@ -86,20 +87,103 @@ or no data are stored in the modified structures at the moment. The theory about how to apply functions a safe way is rather complex. The aim is to define a so-called consistency model. It attempts to define conditions when the new implementation could be used so that the system -stays consistent. The theory is not yet finished. See the discussion at -http://thread.gmane.org/gmane.linux.kernel/1823033/focus=1828189 - -The current consistency model is very simple. It guarantees that either -the old or the new function is called. But various functions get redirected -one by one without any synchronization. - -In other words, the current implementation _never_ modifies the behavior -in the middle of the call. It is because it does _not_ rewrite the entire -function in the memory. Instead, the function gets redirected at the -very beginning. But this redirection is used immediately even when -some other functions from the same patch have not been redirected yet. - -See also the section "Limitations" below. +stays consistent. + +Livepatch has a consistency model which is a hybrid of kGraft and +kpatch: it uses kGraft's per-task consistency and syscall barrier +switching combined with kpatch's stack trace switching. There are also +a number of fallback options which make it quite flexible. + +Patches are applied on a per-task basis, when the task is deemed safe to +switch over. When a patch is enabled, livepatch enters into a +transition state where tasks are converging to the patched state. +Usually this transition state can complete in a few seconds. The same +sequence occurs when a patch is disabled, except the tasks converge from +the patched state to the unpatched state. + +An interrupt handler inherits the patched state of the task it +interrupts. The same is true for forked tasks: the child inherits the +patched state of the parent. + +Livepatch uses several complementary approaches to determine when it's +safe to patch tasks: + +1. The first and most effective approach is stack checking of sleeping + tasks. If no affected functions are on the stack of a given task, + the task is patched. In most cases this will patch most or all of + the tasks on the first try. Otherwise it'll keep trying + periodically. This option is only available if the architecture has + reliable stacks (CONFIG_RELIABLE_STACKTRACE and + CONFIG_STACK_VALIDATION). + +2. The second approach, if needed, is kernel exit switching. A + task is switched when it returns to user space from a system call, a + user space IRQ, or a signal. It's useful in the following cases: + + a) Patching I/O-bound user tasks which are sleeping on an affected + function. In this case you have to send SIGSTOP and SIGCONT to + force it to exit the kernel and be patched. See below - + b) Patching CPU-bound user tasks. If the task is highly CPU-bound + then it will get patched the next time it gets interrupted by an + IRQ. + c) Applying patches for architectures which don't yet have + CONFIG_RELIABLE_STACKTRACE. In this case you'll have to signal + most of the tasks on the system. However this isn't a complete + solution, because there's currently no way to patch kthreads + without CONFIG_RELIABLE_STACKTRACE. + + Note: since idle "swapper" tasks don't ever exit the kernel, they + instead have a kpatch_patch_task() call in the idle loop which allows + them to patched before the CPU enters the idle state. + +3. A third approach (not yet implemented) is planned for the case where + a kthread is sleeping on an affected function. In that case we could + kick the kthread with a signal and then try to patch the task from + the to-be-patched function's livepatch ftrace handler when it + re-enters the function. This will require + CONFIG_RELIABLE_STACKTRACE. + +All the above approaches may be skipped by setting the 'immediate' flag +in the 'klp_patch' struct, which will patch all tasks immediately. This +can be useful if the patch doesn't change any function or data +semantics. Note that, even
