Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755559AbcDNNXY (ORCPT ); Thu, 14 Apr 2016 09:23:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52774 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755452AbcDNNXV (ORCPT ); Thu, 14 Apr 2016 09:23:21 -0400 Date: Thu, 14 Apr 2016 08:23:19 -0500 From: Josh Poimboeuf To: Miroslav Benes Cc: Jiri Kosina , Jessica Yu , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Vojtech Pavlik Subject: Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel Message-ID: <20160414132319.fbvlnogcdeu23vum@treble> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2811 Lines: 69 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 > > --- > > 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 > > #include > > #include > > +#include > > > > #include > > #include > > @@ -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