From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753672AbbCIJQU (ORCPT ); Mon, 9 Mar 2015 05:16:20 -0400 Received: from cantor2.suse.de ([195.135.220.15]:48283 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653AbbCIJQS (ORCPT ); Mon, 9 Mar 2015 05:16:18 -0400 Date: Mon, 9 Mar 2015 10:16:56 +0100 From: Petr Mladek To: Rusty Russell Cc: Seth Jennings , Josh Poimboeuf , Jiri Kosina , Miroslav Benes , Masami Hiramatsu , mingo@kernel.org, mathieu.desnoyers@efficios.com, oleg@redhat.com, paulmck@linux.vnet.ibm.com, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, andi@firstfloor.org, rostedt@goodmis.org, tglx@linutronix.de Subject: Re: [PATCH v2 2/2] livepatch/module: Correctly handle going modules Message-ID: <20150309091656.GF9162@pathway.suse.cz> References: <1425570314-23675-1-git-send-email-pmladek@suse.cz> <1425570314-23675-3-git-send-email-pmladek@suse.cz> <87a8zpy5wb.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a8zpy5wb.fsf@rustcorp.com.au> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat 2015-03-07 11:34:36, Rusty Russell wrote: > Petr Mladek writes: > > Existing live patches are removed from going modules using a notify handler. > > There are two problems with the current implementation. > > > > First, new patch could still see the module in the GOING state even after > > the notifier has been called. It will try to initialize the related > > object structures but the module could disappear at any time. There will > > stay mess in the structures. It might even cause an invalid memory access. > > > > Second, if we start supporting patches with semantic changes between function > > calls, we would need to apply any new patch even for going modules. Note that > > the code from the module could be called even in the GOING state until > > mod->exit() finishes. See below for example. > > I don't think you should handle going modules at all. Rarely happens, > and it should happen fast. I would like to handle it correctly. It would be pity to break a system just because of a module removal. Also the extra overhead will be very small and it will happen only very rarely. We will apply one new patch and remove it quickly after that. But this will happen only when a module is removed and a patch is added at at the "same" time. > If you can hold the module_lock, the easiest thing to do is have us wake > module_wq when a module is freed, then you can just: Unfortunately, we could not use a waitqueue easily. We would need to release klp_mutex to do not block going modules. But we could not do so in the middle of a patch adding. BTW: It seems that module_wq is used for coming modules. We could not use it for coming modules from the same reason. In addition, waiters are weaken after mod->init(). But we would need to apply the patch before mod->init() to avoid any inconsistency. Anyway, thanks for feedback. Best Regards, Petr > retry: > err = wait_event_interruptible(module_wq, > !modules_unloading()); > if (err) > goto out; > > /* Now re-check under lock. */ > mutex_lock(&module_lock); > if (unlikely(modules_unloading()) { > mutex_unlock(&module_lock); > goto retry; > } > > Cheers, > Rusty. > >