Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753101AbbBJRNA (ORCPT ); Tue, 10 Feb 2015 12:13:00 -0500 Received: from casper.infradead.org ([85.118.1.10]:60720 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751551AbbBJRM7 (ORCPT ); Tue, 10 Feb 2015 12:12:59 -0500 Date: Tue, 10 Feb 2015 18:12:55 +0100 From: Peter Zijlstra To: Dave Jones , Linux Kernel Cc: Rusty Russell Subject: Re: [PATCH] module: Annotate nested sleep in resolve_symbol() Message-ID: <20150210171255.GQ24151@twins.programming.kicks-ass.net> References: <20150210164217.GA5547@codemonkey.org.uk> <20150210170604.GA21418@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150210170604.GA21418@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2728 Lines: 89 On which, we should probably do this. --- Subject: module: Replace over-engineered nested sleep Since the introduction of the nested sleep warning; we've established that the occasional sleep inside a wait_event() is fine. wait_event() loops are invariant wrt. spurious wakeups, and the occasional sleep has a similar effect on them. As long as its occasional its harmless. Therefore replace the 'correct' but verbose wait_woken() thing with a simple annotation to shut up the warning. Cc: Rusty Russell Signed-off-by: Peter Zijlstra (Intel) --- kernel/module.c | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index d856e96a3cce..98231bf59b6e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2978,6 +2978,12 @@ static bool finished_loading(const char *name) struct module *mod; bool ret; + /* + * The module_mutex should not be a heavily contended lock; + * if we get the occasional sleep here, we'll go an extra iteration + * in the wait_event_interruptible(), which is harmless. + */ + sched_annotate_sleep(); mutex_lock(&module_mutex); mod = find_module_all(name, strlen(name), true); ret = !mod || mod->state == MODULE_STATE_LIVE @@ -3120,32 +3126,6 @@ static int may_init_module(void) } /* - * Can't use wait_event_interruptible() because our condition - * 'finished_loading()' contains a blocking primitive itself (mutex_lock). - */ -static int wait_finished_loading(struct module *mod) -{ - DEFINE_WAIT_FUNC(wait, woken_wake_function); - int ret = 0; - - add_wait_queue(&module_wq, &wait); - for (;;) { - if (finished_loading(mod->name)) - break; - - if (signal_pending(current)) { - ret = -ERESTARTSYS; - break; - } - - wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT); - } - remove_wait_queue(&module_wq, &wait); - - return ret; -} - -/* * We try to place it in the list now to make sure it's unique before * we dedicate too many resources. In particular, temporary percpu * memory exhaustion. @@ -3165,8 +3145,8 @@ static int add_unformed_module(struct module *mod) || old->state == MODULE_STATE_UNFORMED) { /* Wait in case it fails to load. */ mutex_unlock(&module_mutex); - - err = wait_finished_loading(mod); + err = wait_event_interruptible(module_wq, + finished_loading(mod->name)); if (err) goto out_unlocked; goto again; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/