Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1378610 > unrolled thread
| Started by | Miroslav Benes <mbenes@suse.cz> |
|---|---|
| First post | 2016-04-14 10:50 +0200 |
| Last post | 2016-04-14 15:30 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel Miroslav Benes <mbenes@suse.cz> - 2016-04-14 10:50 +0200
Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel Miroslav Benes <mbenes@suse.cz> - 2016-04-14 11:00 +0200
Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-14 15:50 +0200
Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-14 15:30 +0200
| From | Miroslav Benes <mbenes@suse.cz> |
|---|---|
| Date | 2016-04-14 10:50 +0200 |
| Subject | Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel |
| Message-ID | <rnL8n-7nM-25@gated-at.bofh.it> |
On Fri, 25 Mar 2016, Josh Poimboeuf wrote:
> Update a tasks's universe when returning from a system call or user
> space interrupt, or after handling a signal.
>
> This greatly increases the chances of a patch operation succeeding. If
> a task is I/O bound, it can switch universes when returning from a
> system call. If a task is CPU bound, it can switch universes when
> returning from an interrupt. If a task is sleeping on a to-be-patched
> function, the user can send SIGSTOP and SIGCONT to force it to switch.
>
> Since the idle "swapper" tasks don't ever exit the kernel, they're
> updated from within the idle loop.
Well, I am still not familiarized enough with Andy's recent rework of
entry stuff, but I think all of this is correct. Maybe I would add
a note to the changelog, that since TIF_KLP_NEED_UPDATE is defined 14th
bit it is also automatically included in _TIF_ALLWORK_MASKS.
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> arch/x86/entry/common.c | 6 +++++-
> arch/x86/include/asm/thread_info.h | 2 ++
> include/linux/livepatch.h | 2 ++
> kernel/livepatch/transition.c | 37 +++++++++++++++++++++++++++++++++----
> kernel/sched/idle.c | 4 ++++
> 5 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index e79d93d..94639dd 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -21,6 +21,7 @@
> #include <linux/context_tracking.h>
> #include <linux/user-return-notifier.h>
> #include <linux/uprobes.h>
> +#include <linux/livepatch.h>
>
> #include <asm/desc.h>
> #include <asm/traps.h>
> @@ -202,7 +203,7 @@ long syscall_trace_enter(struct pt_regs *regs)
>
> #define EXIT_TO_USERMODE_LOOP_FLAGS \
> (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
> - _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
> + _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY | _TIF_KLP_NEED_UPDATE)
>
> static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> {
> @@ -236,6 +237,9 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> if (cached_flags & _TIF_USER_RETURN_NOTIFY)
> fire_user_return_notifiers();
>
> + if (unlikely(cached_flags & _TIF_KLP_NEED_UPDATE))
> + klp_update_task_universe(current);
> +
There is a comment at the beginning of this function which should be
updated as well I think.
Miroslav
[toc] | [next] | [standalone]
| From | Miroslav Benes <mbenes@suse.cz> |
|---|---|
| Date | 2016-04-14 11:00 +0200 |
| Message-ID | <rnLi2-7rI-1@gated-at.bofh.it> |
| In reply to | #1378610 |
On Thu, 14 Apr 2016, Miroslav Benes wrote: > On Fri, 25 Mar 2016, Josh Poimboeuf wrote: > > > Update a tasks's universe when returning from a system call or user > > space interrupt, or after handling a signal. > > > > This greatly increases the chances of a patch operation succeeding. If > > a task is I/O bound, it can switch universes when returning from a > > system call. If a task is CPU bound, it can switch universes when > > returning from an interrupt. If a task is sleeping on a to-be-patched > > function, the user can send SIGSTOP and SIGCONT to force it to switch. > > > > Since the idle "swapper" tasks don't ever exit the kernel, they're > > updated from within the idle loop. > > Well, I am still not familiarized enough with Andy's recent rework of > entry stuff, but I think all of this is correct. Maybe I would add > a note to the changelog, that since TIF_KLP_NEED_UPDATE is defined 14th > bit it is also automatically included in _TIF_ALLWORK_MASKS. And I forgot to add that I would try to prepare similar thing for s390 and maybe powerpc (taking recent development there into account). That's gonna be fun :) Miroslav
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-14 15:50 +0200 |
| Message-ID | <rnPOH-2pl-21@gated-at.bofh.it> |
| In reply to | #1378611 |
On Thu, Apr 14, 2016 at 10:50:28AM +0200, Miroslav Benes wrote: > On Thu, 14 Apr 2016, Miroslav Benes wrote: > > > On Fri, 25 Mar 2016, Josh Poimboeuf wrote: > > > > > Update a tasks's universe when returning from a system call or user > > > space interrupt, or after handling a signal. > > > > > > This greatly increases the chances of a patch operation succeeding. If > > > a task is I/O bound, it can switch universes when returning from a > > > system call. If a task is CPU bound, it can switch universes when > > > returning from an interrupt. If a task is sleeping on a to-be-patched > > > function, the user can send SIGSTOP and SIGCONT to force it to switch. > > > > > > Since the idle "swapper" tasks don't ever exit the kernel, they're > > > updated from within the idle loop. > > > > Well, I am still not familiarized enough with Andy's recent rework of > > entry stuff, but I think all of this is correct. Maybe I would add > > a note to the changelog, that since TIF_KLP_NEED_UPDATE is defined 14th > > bit it is also automatically included in _TIF_ALLWORK_MASKS. > > And I forgot to add that I would try to prepare similar thing for s390 and > maybe powerpc (taking recent development there into account). That's gonna > be fun :) Yeah, good point. I've glanced at the entry code for both architectures and I don't think it'll be too bad, though the devil's in the details. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-14 15:30 +0200 |
| Message-ID | <rnPvl-2fL-25@gated-at.bofh.it> |
| In reply to | #1378610 |
On Thu, Apr 14, 2016 at 10:47:04AM +0200, Miroslav Benes wrote:
> On Fri, 25 Mar 2016, Josh Poimboeuf wrote:
>
> > Update a tasks's universe when returning from a system call or user
> > space interrupt, or after handling a signal.
> >
> > This greatly increases the chances of a patch operation succeeding. If
> > a task is I/O bound, it can switch universes when returning from a
> > system call. If a task is CPU bound, it can switch universes when
> > returning from an interrupt. If a task is sleeping on a to-be-patched
> > function, the user can send SIGSTOP and SIGCONT to force it to switch.
> >
> > Since the idle "swapper" tasks don't ever exit the kernel, they're
> > updated from within the idle loop.
>
> Well, I am still not familiarized enough with Andy's recent rework of
> entry stuff, but I think all of this is correct. Maybe I would add
> a note to the changelog, that since TIF_KLP_NEED_UPDATE is defined 14th
> bit it is also automatically included in _TIF_ALLWORK_MASKS.
To be honest, putting it in the range of _TIF_ALLWORK_MASK was an
accident. I think the comments in thread_info.h need to be improved a
bit to make that clearer.
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> > arch/x86/entry/common.c | 6 +++++-
> > arch/x86/include/asm/thread_info.h | 2 ++
> > include/linux/livepatch.h | 2 ++
> > kernel/livepatch/transition.c | 37 +++++++++++++++++++++++++++++++++----
> > kernel/sched/idle.c | 4 ++++
> > 5 files changed, 46 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index e79d93d..94639dd 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -21,6 +21,7 @@
> > #include <linux/context_tracking.h>
> > #include <linux/user-return-notifier.h>
> > #include <linux/uprobes.h>
> > +#include <linux/livepatch.h>
> >
> > #include <asm/desc.h>
> > #include <asm/traps.h>
> > @@ -202,7 +203,7 @@ long syscall_trace_enter(struct pt_regs *regs)
> >
> > #define EXIT_TO_USERMODE_LOOP_FLAGS \
> > (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
> > - _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
> > + _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY | _TIF_KLP_NEED_UPDATE)
> >
> > static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> > {
> > @@ -236,6 +237,9 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> > if (cached_flags & _TIF_USER_RETURN_NOTIFY)
> > fire_user_return_notifiers();
> >
> > + if (unlikely(cached_flags & _TIF_KLP_NEED_UPDATE))
> > + klp_update_task_universe(current);
> > +
>
> There is a comment at the beginning of this function which should be
> updated as well I think.
Yeah, agreed.
--
Josh
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web