From patchwork Sun Aug 31 09:03:20 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Luis R. Rodriguez" X-Patchwork-Id: 384532 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 686DF140182 for ; Sun, 31 Aug 2014 19:04:46 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751426AbaHaJEX (ORCPT ); Sun, 31 Aug 2014 05:04:23 -0400 Received: from mail-pa0-f43.google.com ([209.85.220.43]:43363 "EHLO mail-pa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751244AbaHaJDv (ORCPT ); Sun, 31 Aug 2014 05:03:51 -0400 Received: by mail-pa0-f43.google.com with SMTP id et14so9887884pad.2 for ; Sun, 31 Aug 2014 02:03:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=WnVq4zNzJPq5Be0P4p0bbwG+CwHtAK6RmqQNmEWqQkk=; b=EG6QMuoVm6TWoybgaF7rklf+YKKNYzHPSmiwNg4X1qQEE7jtx8tfVPclDdEbJM+CoH Y+G0xWyDXOHjrUi9drAJX+AtO7LMwBHFyHmIUXYcFPAcd3eyBjRP6y/ktpy1MXC9OOOA 7REBtr7L2azYaQyhvfJ6Ufk1yL1TGLyPG1xDYYSwZwDjJeXLYqTaTrO8s0ZuZnHv6D5B jmvuGeSPiIipubTzS8+1IbaIoxAXKZ8WfoBjqWtsLKOlkVsI8ffhxkm/1QtL5wDbi1hA kqR4bIA2vwMBzm27rXVfJfASuZWC9RmTwG626ei6DN0pQrdw3ksscgrFFvQanLAb2X3l UDtw== X-Received: by 10.68.204.134 with SMTP id ky6mr29967501pbc.61.1409475830067; Sun, 31 Aug 2014 02:03:50 -0700 (PDT) Received: from mcgrof@gmail.com (c-98-234-145-61.hsd1.ca.comcast.net. [98.234.145.61]) by mx.google.com with ESMTPSA id yr3sm16810500pac.1.2014.08.31.02.03.46 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Sun, 31 Aug 2014 02:03:48 -0700 (PDT) Received: by mcgrof@gmail.com (sSMTP sendmail emulation); Sun, 31 Aug 2014 02:03:44 -0700 From: "Luis R. Rodriguez" To: gregkh@linuxfoundation.org, dmitry.torokhov@gmail.com, falcon@meizu.com, tiwai@suse.de, tj@kernel.org, arjan@linux.intel.com Cc: linux-kernel@vger.kernel.org, oleg@redhat.com, akpm@linux-foundation.org, penguin-kernel@i-love.sakura.ne.jp, joseph.salisbury@canonical.com, bpoirier@suse.de, "Luis R. Rodriguez" , Tetsuo Handa , Kay Sievers , One Thousand Gnomes , Tim Gardner , Pierre Fersing , Nagalakshmi Nandigama , Praveen Krishnamoorthy , Sreekanth Reddy , Abhijit Mahajan , Casey Leedom , Hariprasad S , Santosh Rastapur , MPT-FusionLinux.pdl@avagotech.com, linux-scsi@vger.kernel.org, netdev@vger.kernel.org Subject: [RFC v1 3/3] async: add driver asynch levels Date: Sun, 31 Aug 2014 02:03:20 -0700 Message-Id: <1409475800-17573-4-git-send-email-mcgrof@do-not-panic.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1409475800-17573-1-git-send-email-mcgrof@do-not-panic.com> References: <1409475800-17573-1-git-send-email-mcgrof@do-not-panic.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: "Luis R. Rodriguez" Joseph bisected and found that Tetsuo Handa's commit 786235ee "kthread: make kthread_create() killable" modified kthread_create() to bail as soon as SIGKILL is received [0] [1]. This is causing some issues with some drivers and at times boot. There are other patches which could also enable the SIGKILL trigger on driver loading though: 70834d30 "usermodehelper: use UMH_WAIT_PROC consistently" b3449922 "usermodehelper: introduce umh_complete(sub_info)" d0bd587a "usermodehelper: implement UMH_KILLABLE" 9d944ef3 "usermodehelper: kill umh_wait, renumber UMH_* constants" 5b9bd473 "usermodehelper: ____call_usermodehelper() doesn't need do_exit()" 3e63a93b "kmod: introduce call_modprobe() helper" 1cc684ab "kmod: make __request_module() killable" All of these went in on 3.4 upstream, and were part of the fixes for CVE-2012-4398 [2] and documented more properly on Red Hat's bugzilla [3]. Any of these patches may contribute to having a module be properly killed now, but 786235ee is the latest in the series. For instance on SLE12 cxgb4 has been fond to get the SIGKILL even though SLE12 does not yet have 786235ee merged [4]. Joseph found that the systemd-udevd process sends SIGKILL to systemd's usage of kmod for module loading if probe on a driver takes over 30 seconds [5] [6]. When this happens probe will fail on any driver, its why booting on some systems will fail if the driver happens to be a storage related driver. When helping debug the issue Tetsuo suggested fixing this issue by modifying kthread_create() to not leave upon SIGKILL immediately if the source of the SIGKILL was the OOM, and actually wait for 10 seconds more before completing the kill [7]. This work around would in turn only help by adding an extra 10 second delay increasing in effect the systemd timeout by an extra 10 seconds. Drivers which take more than 40 seconds should then still fail to load on kernels with this work around patch. Upon review of this patch Oleg rejected this change [8] and the discussion was punted out to systemd-devel to see if the default timeout could be increased from 30 seconds to 120 [9]. The opinion of the systemd maintainers was that the driver's behavior should be fixed [10]. Linus seems to agree [11], however more recently even networking drivers have been reported to fail on probe since just writing the firmware to a device and kicking it can take easy over 60 seconds [4]. Benjamim was able to trace the issues recently reported on cxgb4 down to the same systemd-udevd 30 second timeout [6]. Folks are a bit confused here though -- its not only module initialization which is being penalized, because the driver core will immediately trigger the driver's own bus probe routine if autoprobe is enabled each driver's probe routine must also complete within the same 30 second timeout. This means not only should driver's init complete within the set default systemd timeout of 30 seconds but so should the probe routine, and probe would obviously also have less time given that the timeout is for both the module's init() and its own bus' probe(). Quite a bit of driver's fail to complete the bus' probe within 30 seconds, its not the init routine that takes long. We'll need a solution to split up asynch probing then. This solution provides a more generic module-agnostic solution which could be used by any init() caller and ends up respecting the same init levels as when things are built-in. [0] http://thread.gmane.org/gmane.linux.ubuntu.devel.kernel.general/39123 [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1276705 [2] http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2012-4398 [3] https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2012-4398 [4] https://bugzilla.novell.com/show_bug.cgi?id=877622 [5] http://article.gmane.org/gmane.linux.kernel/1669550 [6] https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1297248 [7] https://launchpadlibrarian.net/169657493/kthread-defer-leaving.patch [8] http://article.gmane.org/gmane.linux.kernel/1669604 [9] http://lists.freedesktop.org/archives/systemd-devel/2014-March/018006.html [10] http://article.gmane.org/gmane.comp.sysutils.systemd.devel/17860 [11] http://article.gmane.org/gmane.linux.kernel/1671333 Cc: Tejun Heo Cc: Arjan van de Ven Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Joseph Salisbury Cc: Kay Sievers Cc: One Thousand Gnomes Cc: Tim Gardner Cc: Pierre Fersing Cc: Andrew Morton Cc: Oleg Nesterov Cc: Benjamin Poirier Cc: Nagalakshmi Nandigama Cc: Praveen Krishnamoorthy Cc: Sreekanth Reddy Cc: Abhijit Mahajan Cc: Casey Leedom Cc: Hariprasad S Cc: Santosh Rastapur Cc: MPT-FusionLinux.pdl@avagotech.com Cc: linux-scsi@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: netdev@vger.kernel.org Signed-off-by: Luis R. Rodriguez --- include/linux/async.h | 4 ++++ include/linux/init.h | 34 ++++++++++++++++++++++++++++++++++ kernel/async.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/include/linux/async.h b/include/linux/async.h index 6b0226b..e06544b 100644 --- a/include/linux/async.h +++ b/include/linux/async.h @@ -40,9 +40,13 @@ struct async_domain { extern async_cookie_t async_schedule(async_func_t func, void *data); extern async_cookie_t async_schedule_domain(async_func_t func, void *data, struct async_domain *domain); +extern async_cookie_t async_schedule_level(async_func_t func, void *data, + int level); + void async_unregister_domain(struct async_domain *domain); extern void async_synchronize_full(void); extern void async_synchronize_full_domain(struct async_domain *domain); +extern void async_synchronize_level(int level); extern void async_synchronize_cookie(async_cookie_t cookie); extern void async_synchronize_cookie_domain(async_cookie_t cookie, struct async_domain *domain); diff --git a/include/linux/init.h b/include/linux/init.h index 3b69b1a..6ba7e4f 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -282,6 +282,7 @@ void __init parse_early_options(char *cmdline); * be one per module. */ #define module_init(x) __initcall(x); +#define module_init_async(x) __initcall(x); /** * module_exit() - driver exit entry point @@ -294,9 +295,12 @@ void __init parse_early_options(char *cmdline); * There can only be one per module. */ #define module_exit(x) __exitcall(x); +#define module_exit_async(x) __exitcall(x); #else /* MODULE */ +#include + /* * In most cases loadable modules do not need custom * initcall levels. There are still some valid cases where @@ -335,9 +339,39 @@ void __init parse_early_options(char *cmdline); { return exitfn; } \ void cleanup_module(void) __attribute__((alias(#exitfn))); +#define drv_init_async(initfn, __level) \ + static int ___init_ret; \ + static void _drv_init_async_##initfn(void *data, async_cookie_t cookie) \ + { \ + initcall_t fn = data; \ + async_synchronize_level(__level - 1); \ + ___init_ret = fn(); \ + if (___init_ret !=0) \ + printk(KERN_DEBUG \ + "async init routine failed: " #initfn "(): %d\n", ___init_ret); \ + } \ + static __init int __drv_init_async_##initfn(void) \ + { \ + async_schedule_level(_drv_init_async_##initfn, initfn, __level); \ + return 0; \ + } \ + drv_init(__drv_init_async_##initfn); + +#define drv_exit_async(exitfn, level) \ + static __exit void __drv_exit_async##exitfn(void) \ + { \ + async_synchronize_level(level); \ + if (___init_ret == 0) \ + exitfn(); \ + } \ + drv_exit(__drv_exit_async##exitfn); + #define module_init(initfn) drv_init(initfn); #define module_exit(exitfn) drv_exit(exitfn); +#define module_init_async(fn) drv_init_async(fn, 7) +#define module_exit_async(exitfn) drv_exit_async(exitfn, 7) + #define __setup_param(str, unique_id, fn) /* nothing */ #define __setup(str, func) /* nothing */ #endif diff --git a/kernel/async.c b/kernel/async.c index 362b3d6..4d80a36 100644 --- a/kernel/async.c +++ b/kernel/async.c @@ -68,6 +68,20 @@ static LIST_HEAD(async_global_pending); /* pending from all registered doms */ static ASYNC_DOMAIN(async_dfl_domain); static DEFINE_SPINLOCK(async_lock); +#define ASYNC_DOMAIN_LEVEL(level) \ + { .pending = LIST_HEAD_INIT(async_level_domains[level-1].pending), \ + .registered = 0 } + +static struct async_domain async_level_domains[] = { + ASYNC_DOMAIN_LEVEL(1), + ASYNC_DOMAIN_LEVEL(2), + ASYNC_DOMAIN_LEVEL(3), + ASYNC_DOMAIN_LEVEL(4), + ASYNC_DOMAIN_LEVEL(5), + ASYNC_DOMAIN_LEVEL(6), + ASYNC_DOMAIN_LEVEL(7), +}; + struct async_entry { struct list_head domain_list; struct list_head global_list; @@ -237,6 +251,14 @@ async_cookie_t async_schedule_domain(async_func_t func, void *data, } EXPORT_SYMBOL_GPL(async_schedule_domain); +async_cookie_t async_schedule_level(async_func_t func, void *data, int level) +{ + if (level <= 0 || level > ARRAY_SIZE(async_level_domains)) + return __async_schedule_sync(func, data); + return async_schedule_domain(func, data, &async_level_domains[level-1]); +} +EXPORT_SYMBOL_GPL(async_schedule_level); + /** * async_synchronize_full - synchronize all asynchronous function calls * @@ -279,6 +301,17 @@ void async_synchronize_full_domain(struct async_domain *domain) } EXPORT_SYMBOL_GPL(async_synchronize_full_domain); +void async_synchronize_level(int level) +{ + int i; + if (level <= 0 || level > ARRAY_SIZE(async_level_domains)) + return; + + for (i=1; i <= level; i++) + async_synchronize_full_domain(&async_level_domains[i-1]); +} +EXPORT_SYMBOL_GPL(async_synchronize_level); + /** * async_synchronize_cookie_domain - synchronize asynchronous function calls within a certain domain with cookie checkpointing * @cookie: async_cookie_t to use as checkpoint