From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3tyT7M29B3zDq66 for ; Tue, 10 Jan 2017 21:40:15 +1100 (AEDT) Date: Tue, 10 Jan 2017 11:40:04 +0100 From: Petr Mladek To: Josh Poimboeuf Cc: Miroslav Benes , Jessica Yu , Jiri Kosina , Linux Kernel Mailing List , live-patching@vger.kernel.org, Michael Ellerman , Heiko Carstens , x86@kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, Vojtech Pavlik , Jiri Slaby , Chris J Arges , Andy Lutomirski , Ingo Molnar , Peter Zijlstra Subject: Re: [PATCH v3 13/15] livepatch: change to a per-task consistency model Message-ID: <20170110104004.GG20785@pathway.suse.cz> References: <20161220173246.GC25166@pathway.suse.cz> <20161221212505.dbxeddu2skmjmwiq@treble> <20161223101803.GB2541@linux.suse> <20170106200734.d7wvry4upz2ffwem@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20170106200734.d7wvry4upz2ffwem@treble> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri 2017-01-06 14:07:34, Josh Poimboeuf wrote: > On Fri, Dec 23, 2016 at 11:18:03AM +0100, Petr Mladek wrote: > > On Fri 2016-12-23 10:24:35, Miroslav Benes wrote: > > > > > > diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c > > > > > > index 5efa262..e79ebb5 100644 > > > > > > --- a/kernel/livepatch/patch.c > > > > > > +++ b/kernel/livepatch/patch.c > > > > > > @@ -29,6 +29,7 @@ > > > > > > #include > > > > > > #include > > > > > > #include "patch.h" > > > > > > +#include "transition.h" > > > > > > > > > > > > static LIST_HEAD(klp_ops); > > > > > > > > > > > > @@ -54,15 +55,53 @@ static void notrace klp_ftrace_handler(unsigned long ip, > > > > > > { > > > > > > struct klp_ops *ops; > > > > > > struct klp_func *func; > > > > > > + int patch_state; > > > > > > > > > > > > ops = container_of(fops, struct klp_ops, fops); > > > > > > > > > > > > rcu_read_lock(); > > > > > > + > > > > > > func = list_first_or_null_rcu(&ops->func_stack, struct klp_func, > > > > > > stack_node); > > > > > > - if (WARN_ON_ONCE(!func)) > > > > > > + > > > > > > + if (!func) > > > > > > goto unlock; > > > > > > Yeah, I'm thinking we should keep the warning to catch any bugs in case > any of our ftrace assumptions change. Maybe I should add a comment: > > /* > * func can never be NULL because preemption should be disabled > * here and unregister_ftrace_function() does the equivalent of > * a synchronize_sched() before the func_stack removal. > */ > if (WARN_ON_ONCE(!func)) > goto unlock; Sounds reasonable to me. Best Regards, Petr