From patchwork Thu Jun 1 06:21:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 9758725 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 BF5DE60375 for ; Thu, 1 Jun 2017 06:21:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B0F2F284DC for ; Thu, 1 Jun 2017 06:21:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A5A41284F5; Thu, 1 Jun 2017 06:21:37 +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 63720284DC for ; Thu, 1 Jun 2017 06:21:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751272AbdFAGVg (ORCPT ); Thu, 1 Jun 2017 02:21:36 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:35670 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750848AbdFAGVf (ORCPT ); Thu, 1 Jun 2017 02:21:35 -0400 Received: from [192.168.1.66] (75-37-194-224.lightspeed.lsatca.sbcglobal.net [75.37.194.224]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id B0DCB1740F8; Thu, 1 Jun 2017 06:24:29 +0000 (UTC) Message-ID: <1496298093.27407.203.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH] iscsi: Fix a sleep-in-atomic bug From: "Nicholas A. Bellinger" To: Jia-Ju Bai Cc: bart.vanassche@sandisk.com, davem@davemloft.net, hare@suse.com, elfring@users.sourceforge.net, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org, linux-kernel@vger.kernel.org Date: Wed, 31 May 2017 23:21:33 -0700 In-Reply-To: <1496201199-3661-1-git-send-email-baijiaju1990@163.com> References: <1496201199-3661-1-git-send-email-baijiaju1990@163.com> X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Jia-Ju, On Wed, 2017-05-31 at 11:26 +0800, Jia-Ju Bai wrote: > The driver may sleep under a spin lock, and the function call path is: > iscsit_tpg_enable_portal_group (acquire the lock by spin_lock) > iscsi_update_param_value > kstrdup(GFP_KERNEL) --> may sleep > > To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". > > Signed-off-by: Jia-Ju Bai > --- > drivers/target/iscsi/iscsi_target_parameters.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Btw, the use of tpg->tpg_state_lock in iscsit_tpg_enable_portal_group() while checking existing state and calling iscsi_update_param_value() is not necessary, since lio_target_tpg_enable_store() is already holding iscsit_get_tpg() -> tpg->tpg_access_lock. How about the following instead to only take tpg->tpg_state_lock when updating tpg->tpg_state instead..? diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c index 2e7e08d..abaabba 100644 --- a/drivers/target/iscsi/iscsi_target_tpg.c +++ b/drivers/target/iscsi/iscsi_target_tpg.c @@ -311,11 +311,9 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) struct iscsi_tiqn *tiqn = tpg->tpg_tiqn; int ret; - spin_lock(&tpg->tpg_state_lock); if (tpg->tpg_state == TPG_STATE_ACTIVE) { pr_err("iSCSI target portal group: %hu is already" " active, ignoring request.\n", tpg->tpgt); - spin_unlock(&tpg->tpg_state_lock); return -EINVAL; } /* @@ -324,10 +322,8 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) * is enforced (as per default), and remove the NONE option. */ param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list); - if (!param) { - spin_unlock(&tpg->tpg_state_lock); + if (!param) return -EINVAL; - } if (tpg->tpg_attrib.authentication) { if (!strcmp(param->value, NONE)) { @@ -341,6 +337,7 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) goto err; } + spin_lock(&tpg->tpg_state_lock); tpg->tpg_state = TPG_STATE_ACTIVE; spin_unlock(&tpg->tpg_state_lock); @@ -353,7 +350,6 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) return 0; err: - spin_unlock(&tpg->tpg_state_lock); return ret; }