From patchwork Wed Feb 22 02:09:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 9586039 X-Patchwork-Delegate: luca@coelho.fi Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 56D0B60578 for ; Wed, 22 Feb 2017 02:10:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3AFCC2868D for ; Wed, 22 Feb 2017 02:10:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2FB5628693; Wed, 22 Feb 2017 02:10:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D8C5928692 for ; Wed, 22 Feb 2017 02:10:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932096AbdBVCKO (ORCPT ); Tue, 21 Feb 2017 21:10:14 -0500 Received: from mail.kernel.org ([198.145.29.136]:55428 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754019AbdBVCJv (ORCPT ); Tue, 21 Feb 2017 21:09:51 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 735E1201C8; Wed, 22 Feb 2017 02:09:49 +0000 (UTC) Received: from garbanzo.do-not-panic.com (c-73-15-241-2.hsd1.ca.comcast.net [73.15.241.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 3B3C820256; Wed, 22 Feb 2017 02:09:48 +0000 (UTC) From: "Luis R. Rodriguez" To: johannes.berg@intel.com, luciano.coelho@intel.com, emmanuel.grumbach@intel.com Cc: ming.lei@canonical.com, zajec5@gmail.com, linuxwifi@intel.com, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, "Luis R. Rodriguez" Subject: [PATCH v2 2/2] iwlwifi: simplify requesting ops module Date: Tue, 21 Feb 2017 18:09:39 -0800 Message-Id: <20170222020939.28140-3-mcgrof@kernel.org> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170222020939.28140-1-mcgrof@kernel.org> References: <20170222001822.GE31264@wotan.suse.de> <20170222020939.28140-1-mcgrof@kernel.org> X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The return value of request_module() being 0 does not mean that the driver which was requested has loaded. To properly check that the driver was loaded each driver can use internal mechanisms to vet the driver is now present. The helper try_then_request_module() was added to help with this, allowing drivers to specify their own validation as the first argument for synchronous requests. On iwlwifi the use case is a bit more complicated given that the value we need to check for is protected with a mutex later used on the module_init() of the module we are asking for. If we were to lock and request_module() we'd deadlock. iwlwifi could use its own wrapper for requesting its ops module synchronously so it can handle its special locking requirements on its own but the given code has been in the kernel for a long time and the preference is to keep its simplicity. We can do two more things to simplify this even further: a) just use the async request, request_module_nowait(), to acknowledge we are using best effort -- we are not verifying the module is loaded b) we can unconditionally call request_module_nowait() whether or not we were built-in or not, if built-in this will return immediately without failure. Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index e198d6f5fcea..e96095c1824a 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -1250,7 +1250,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) size_t trigger_tlv_sz[FW_DBG_TRIGGER_MAX]; u32 api_ver; int i; - bool load_module = false; bool usniffer_images = false; fw->ucode_capa.max_probe_length = IWL_DEFAULT_MAX_PROBE_LENGTH; @@ -1454,8 +1453,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) mutex_unlock(&iwlwifi_opmode_table_mtx); goto out_unbind; } - } else { - load_module = true; } mutex_unlock(&iwlwifi_opmode_table_mtx); @@ -1466,20 +1463,10 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) */ complete(&drv->request_firmware_complete); - /* - * Load the module last so we don't block anything - * else from proceeding if the module fails to load - * or hangs loading. - */ - if (load_module) { - err = request_module("%s", op->name); -#ifdef CONFIG_IWLWIFI_OPMODE_MODULAR - if (err) - IWL_ERR(drv, - "failed to load module %s (error %d), is dynamic loading enabled?\n", - op->name, err); -#endif - } + err = request_module_nowait("%s", op->name); + if (err) + goto out_unbind; + goto free; try_again: