From patchwork Thu Nov 26 13:29:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333978 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1ACAC63697 for ; Thu, 26 Nov 2020 13:30:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8E9EB21D1A for ; Thu, 26 Nov 2020 13:30:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="jjM+rY5r"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="Sjm02VUm" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390050AbgKZNac (ORCPT ); Thu, 26 Nov 2020 08:30:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56752 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389957AbgKZNab (ORCPT ); Thu, 26 Nov 2020 08:30:31 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9CE74C0613D4 for ; Thu, 26 Nov 2020 05:30:31 -0800 (PST) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YAPMvJwZmNtmAWRutyXilXN5I9e5gBSqyvoXiwLJWcs=; b=jjM+rY5rOgFTB1sn+Rvl965x9DveZ0L3RQHvZWN/hutxhAiqhTcpBWhIY0u029oCCy94gz 35JlETmKajxRr6k/Uxzs/Jb2AssdZYO8eHUCmspJiAHIOMnSrgNeqm2L+eN0MFEQXSyU+Y 5GyP5f8SRf+kDiynJ/2U+vjRK8/P8AdZNHY8IGxoJqqDX5jq1dL7kJVbAwncQeftDWcJeo qdOIVnrtYPtdPjsInRhSvI+bxPHZxztl51MfNcK7SyrMxedN7QSd9hNbWT6J5F8YXld6RQ dsEnbtmbKyepPiweR3PyA2EpFyJnQwL++V8+XLs129nCUIuE4ql/ZaGmpNYQFA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YAPMvJwZmNtmAWRutyXilXN5I9e5gBSqyvoXiwLJWcs=; b=Sjm02VUmCAqhWj/5R+sVWw3XNIxGn5WuVdXPW6tEXmdDE7d5I/pbXiFJCx5XfSwXtqjUcx /FVhp1TSXrZ2WwAw== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 01/14] scsi: pm80xx: Do not sleep in atomic context. Date: Thu, 26 Nov 2020 14:29:39 +0100 Message-Id: <20201126132952.2287996-2-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" hw_event_sas_phy_up() is used in hardirq/softirq context: pm8001_interrupt_handler_msix() || pm8001_interrupt_handler_intx() || pm8001_tasklet => PM8001_CHIP_DISP->isr() = pm80xx_chip_isr() => process_oq() [spin_lock_irqsave(&pm8001_ha->lock,)] => process_one_iomb() => mpi_hw_event() => hw_event_sas_phy_up() => msleep(200) Revert the msleep() back to an mdelay() to avoid sleeping in atomic context. Fixes: 4daf1ef3c681 ("scsi: pm80xx: Convert 'long' mdelay to msleep") Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Vikram Auradkar Cc: Jack Wang --- drivers/scsi/pm8001/pm80xx_hwi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 69f8244539e04..7d838e316657c 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -3296,7 +3296,7 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb) pm8001_get_attached_sas_addr(phy, phy->sas_phy.attached_sas_addr); spin_unlock_irqrestore(&phy->sas_phy.frame_rcvd_lock, flags); if (pm8001_ha->flags == PM8001F_RUN_TIME) - msleep(200);/*delay a moment to wait disk to spinup*/ + mdelay(200);/*delay a moment to wait disk to spinup*/ pm8001_bytes_dmaed(pm8001_ha, phy_id); } From patchwork Thu Nov 26 13:29:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333388 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8ECE7C56202 for ; Thu, 26 Nov 2020 13:30:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 42E1121D7E for ; Thu, 26 Nov 2020 13:30:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ymCbyc72"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="56vEOwTk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390057AbgKZNad (ORCPT ); Thu, 26 Nov 2020 08:30:33 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56450 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389955AbgKZNad (ORCPT ); Thu, 26 Nov 2020 08:30:33 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397430; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nCHH+WU06bIildt8jR1f7yAZJ5+Z7ksfyeAxtSEkZIU=; b=ymCbyc72lNOJoMf8MAh/+bYJSFOWyhWk4LQDgL0Bf4p0cxZgu/X9P//oLgEvmaE9j1NlAO iDejMtK/wOQNxEEdQVcEDjwg9Opugw3bs4Rrne8gmv10tN0DpyeWSwWVdq4op2K2fcNVka QsW1YvX5dO6+SohMqlr6BjtyM6r5yPuk7BlHKXcsJSCs2cntkBf2H2/ItzqpUU/EErnkhg PdhL5Lvy4HR6OL2FwXyJI7HLIBzpJryCLuEsT5QDYuhftPDnF/X2xAtQllWAAjhHKbIyPq xPmCUu1nEb6wEaSkQHedS9O3lCBhOnc24+BTWipWFrBghUAAi8CuD5IoOT1hUg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397430; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nCHH+WU06bIildt8jR1f7yAZJ5+Z7ksfyeAxtSEkZIU=; b=56vEOwTkXDyd7Tk8GeC8Gtc6EyM5DiXVrmPEXaoyCiQugDjuuWpIl8J7nmvRle7ku7VUGm VkfdlHk6tRQVJCAg== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 02/14] scsi: hisi_sas: Remove preemptible(). Date: Thu, 26 Nov 2020 14:29:40 +0100 Message-Id: <20201126132952.2287996-3-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" hisi_sas_task_exec() uses preemptible() to see if it's safe to block. This does not work for CONFIG_PREEMPT_COUNT=n kernels in which preemptible() always returns 0. The problem is masked when enabling some of the common Kconfig.debug options (like CONFIG_DEBUG_ATOMIC_SLEEP), as they implicitly enable the preemption counter. In general, driver leaf functions should not make logic decisions based on the context they're called from. The caller should be the entity responsible for explicitly indicating context. Since hisi_sas_task_exec() already has a gfp_t flags parameter, use it as the explicit context marker. Fixes: 214e702d4b70 ("scsi: hisi_sas: Adjust task reject period during host reset") Fixes: 550c0d89d52d ("scsi: hisi_sas: Replace in_softirq() check in hisi_sas_task_exec()") Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Xiaofei Tan Cc: Xiang Chen Cc: John Garry Acked-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_main.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index c8dd8588f800e..06e65c461f027 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -585,13 +585,7 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags, dev = hisi_hba->dev; if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags))) { - /* - * For IOs from upper layer, it may already disable preempt - * in the IO path, if disable preempt again in down(), - * function schedule() will report schedule_bug(), so check - * preemptible() before goto down(). - */ - if (!preemptible()) + if (!gfpflags_allow_blocking(gfp_flags)) return -EINVAL; down(&hisi_hba->sem); From patchwork Thu Nov 26 13:29:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333977 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05D9CC63697 for ; Thu, 26 Nov 2020 13:30:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0A4621D7E for ; Thu, 26 Nov 2020 13:30:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="zE/KQFnW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="I1lZIBbK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389955AbgKZNae (ORCPT ); Thu, 26 Nov 2020 08:30:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56762 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390052AbgKZNad (ORCPT ); Thu, 26 Nov 2020 08:30:33 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00BB5C0617A7 for ; Thu, 26 Nov 2020 05:30:32 -0800 (PST) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397431; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UBYalvQNYVNA2galq9HAG+ZzSJ6S7VbYrFShfgOIJ7k=; b=zE/KQFnW3QuanfbNtQInVp/I+KzCgOTuiDXvRLcj3yShlmc2yE8/PfD0youvT0qAWDHg5m o9AOIE5LJzX2WJaxzW+guSfwVzLStSgS15gM0fuXDqRNHZ6URXAVowI1OaGfGwNPm4NcCQ HdHUI4zeZdXAZU3mRfZESOmqAgGPr8xLqDS4ecJxFoKf4vOtz++ZU+ooILLTUo6hj7eclW 6NmfHuAxQneGzgNvA/T3nsnjt5+dMOVsy++fHzNMccpIX5srU2Nj20BAuihjnMvip/7Dah Nu8STX+gMMowT8E+5ISUy/aTzwXmS1LhmxhQdIIQrOFOSgz9v/HUEpL9SecHhw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397431; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UBYalvQNYVNA2galq9HAG+ZzSJ6S7VbYrFShfgOIJ7k=; b=I1lZIBbKN1385hl/hDFtpJGO+IXWN/TFqnIYz3TNWRKc5piVJ6+Lig9SnVQluE8An6Yb0z 3d65DVjlC+pCaTBA== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 03/14] scsi: qla4xxx: qla4_82xx_crb_win_lock(): Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:41 +0100 Message-Id: <20201126132952.2287996-4-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" qla4_82xx_crb_win_lock() spins on a certain hardware state until it's updated. At the end of each spin, if in_interrupt() is true, it does 20 loops of cpu_relax(). Otherwise, it yields the CPU. The in_interrupt() macro is ill-defined as it does not provide what the name suggests, and it does not catch the intended use-case here. qla4_82xx_crb_win_lock() is always invoked with scsi_qla_host::hw_lock acquired, with disabled interrupts. If the caller is in process context, as in qla4_82xx_need_reset_handler(), then in_interrupt() will return false even though it is not allowed to call schedule(). Remove the in_interrupt() check. Change qla4_82xx_crb_win_lock() specification to a purely atomic function. Mark it as static, remove its forward declaration, and move it above its callers. To avoid hammering the PCI bus while spinning, use a 10 micro-second delay instead of cpu_relax(). Fixes: f4f5df23bf72 ("[SCSI] qla4xxx: Added support for ISP82XX") Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Nilesh Javali Cc: Manish Rangankar Cc: Reviewed-by: Daniel Wagner --- drivers/scsi/qla4xxx/ql4_glbl.h | 1 - drivers/scsi/qla4xxx/ql4_nx.c | 63 +++++++++++++++------------------ 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_glbl.h b/drivers/scsi/qla4xxx/ql4_glbl.h index b8f02210aeb03..ea60057b2e20a 100644 --- a/drivers/scsi/qla4xxx/ql4_glbl.h +++ b/drivers/scsi/qla4xxx/ql4_glbl.h @@ -114,7 +114,6 @@ irqreturn_t qla4_82xx_intr_handler(int irq, void *dev_id); void qla4_82xx_queue_iocb(struct scsi_qla_host *ha); void qla4_82xx_complete_iocb(struct scsi_qla_host *ha); -int qla4_82xx_crb_win_lock(struct scsi_qla_host *); void qla4_82xx_crb_win_unlock(struct scsi_qla_host *); int qla4_82xx_pci_get_crb_addr_2M(struct scsi_qla_host *, ulong *); void qla4_82xx_wr_32(struct scsi_qla_host *, ulong, u32); diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c index f1767b21076f7..da903a545b2c0 100644 --- a/drivers/scsi/qla4xxx/ql4_nx.c +++ b/drivers/scsi/qla4xxx/ql4_nx.c @@ -375,6 +375,35 @@ qla4_82xx_pci_set_crbwindow_2M(struct scsi_qla_host *ha, ulong *off) *off = (*off & MASK(16)) + CRB_INDIRECT_2M + ha->nx_pcibase; } +#define CRB_WIN_LOCK_TIMEOUT 100000000 + +/* + * Context: atomic + */ +static int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) +{ + int done = 0, timeout = 0; + + while (!done) { + /* acquire semaphore3 from PCI HW block */ + done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_LOCK)); + if (done == 1) + break; + if (timeout >= CRB_WIN_LOCK_TIMEOUT) + return -1; + + timeout++; + udelay(10); + } + qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num); + return 0; +} + +void qla4_82xx_crb_win_unlock(struct scsi_qla_host *ha) +{ + qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_UNLOCK)); +} + void qla4_82xx_wr_32(struct scsi_qla_host *ha, ulong off, u32 data) { @@ -475,40 +504,6 @@ int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data) return rval; } -#define CRB_WIN_LOCK_TIMEOUT 100000000 - -int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) -{ - int i; - int done = 0, timeout = 0; - - while (!done) { - /* acquire semaphore3 from PCI HW block */ - done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_LOCK)); - if (done == 1) - break; - if (timeout >= CRB_WIN_LOCK_TIMEOUT) - return -1; - - timeout++; - - /* Yield CPU */ - if (!in_interrupt()) - schedule(); - else { - for (i = 0; i < 20; i++) - cpu_relax(); /*This a nop instr on i386*/ - } - } - qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num); - return 0; -} - -void qla4_82xx_crb_win_unlock(struct scsi_qla_host *ha) -{ - qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_UNLOCK)); -} - #define IDC_LOCK_TIMEOUT 100000000 /** From patchwork Thu Nov 26 13:29:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333386 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B261C64E7D for ; Thu, 26 Nov 2020 13:30:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C503021D40 for ; Thu, 26 Nov 2020 13:30:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="buBUq5lH"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="/2A0sdDG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390070AbgKZNae (ORCPT ); Thu, 26 Nov 2020 08:30:34 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56502 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389957AbgKZNad (ORCPT ); Thu, 26 Nov 2020 08:30:33 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397432; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ISkd7gV5GLtaW758unEPO6qObTdATD8RZJ4kXQk49Fg=; b=buBUq5lH2wBNsm0mh1CFOY5ICwo+EUCNEo2vPfCMW0mlGdTRZGabaFEyI9Wy7jq7Wi5a88 h1MWgmQyVPQnhOy+JIAUBwTQ9rKUiq83sizFhTcLXkaK+F0m+z78cc4XjWzVLc4n1QkuMg NbHCVsF3wk4xJQfJeb3hF7tah+I6PyIH0pclW164Fy1COrklcZTHrD9uvVppNWt0EbNVuW y3/iAXTWxT8UeO6yv26oolpLx3cvzzkomqq6RkZmGMrkOKjiP9F2RWsb/WhabJS2z4Uuum P2k/7HE4LA4xBpjSmuP2s8+M0pXVg+HUnNtIDpL0SYD9cIgcF5HiB4Xn7JHoTw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397432; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ISkd7gV5GLtaW758unEPO6qObTdATD8RZJ4kXQk49Fg=; b=/2A0sdDG3ovvGVDOLX1NfOS7ZR747K6YIFIMFSrFK4ZC3DK9qHK8t2L4vZsEO0kHzkSuG3 SvyerGNClnjeOHAQ== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 04/14] scsi: qla2xxx: qla82xx: Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:42 +0100 Message-Id: <20201126132952.2287996-5-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" qla82xx_idc_lock() spins on a certain hardware state until it's updated. At the end of each spin, if in_interrupt() is true, it does 20 loops of cpu_relax(). Otherwise, it yields the CPU. While in_interrupt() is ill-defined and does not provide what the name suggests, it is not needed here: qla82xx_idc_lock() is always called from process context. Below is an analysis of its callers, in order of appearance: - qla_nx.c: qla82xx_device_bootstrap(), only called by qla82xx_device_state_handler(), has multiple msleep()s. - qla_nx.c: qla82xx_need_qsnt_handler(), has one second msleep() - qla_nx.c: qla82xx_wait_for_state_change(), one second msleep() - qla_nx.c: qla82xx_need_reset_handler(), can sleep up to 10 seconds - qla_nx.c: qla82xx_device_state_handler(), has multiple msleep()s - qla_nx.c: qla82xx_abort_isp(), if it's a qla82xx controller, calls qla82xx_device_state_handler(), which sleeps. It's also bound to isp_operations ->abort_isp() hook, where all the callers are in process context. - qla_nx.c: qla82xx_beacon_on(), bound to isp_operations ->beacon_on() hook. That hook is only called once, in a mutex locked context, from qla2x00_beacon_store(). - qla_nx.c: qla82xx_beacon_off(), bound to isp_operations ->beacon_off() hook. Like ->beacon_on(), it's only called once, in a mutex locked context, from qla2x00_beacon_store(). - qla_nx.c: qla82xx_fw_dump(), calls qla2x00_wait_for_chip_reset(), which has msleep() in a loop. It is bound to isp_operations ->fw_dump() hook. That hook *is* called from atomic context at qla_isr.c by multiple interrupt handlers. Nonetheless, it's other controllers interrupt handlers, and not the qla82xx. - qla_attr.c: qla2x00_sysfs_write_fw_dump(), and qla2x00_sysfs_write_reset(), process-context sysfs ->write() hooks. - qla_os.c: qla2x00_probe_one(). PCI ->probe(), process context. - qla_os.c: qla2x00_clear_drv_active(), called solely from qla2x00_remove_one(), which is PCI ->remove() hook, process context. - qla_os.c: qla2x00_do_dpc(), kthread function, process context. Remove the in_interrupt() check. Change qla82xx_idc_lock() specification to a purely process-context function. Mark it with "Context: task, might sleep". Change qla82xx_idc_lock() implementation to sleep 100ms, instead of a schedule(), for each spin. This is more deterministic, and it matches the other qla models idc_lock() functions. Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Nilesh Javali Cc: Reviewed-by: Daniel Wagner Reviewed-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 5 +++++ drivers/scsi/qla2xxx/qla_nx.c | 25 +++++++++++-------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index ed9b10f8537d6..fe3c0e2f1ce88 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -3296,8 +3296,10 @@ struct isp_operations { void (*fw_dump)(struct scsi_qla_host *vha); void (*mpi_fw_dump)(struct scsi_qla_host *, int); + /* Context: task, might sleep */ int (*beacon_on) (struct scsi_qla_host *); int (*beacon_off) (struct scsi_qla_host *); + void (*beacon_blink) (struct scsi_qla_host *); void *(*read_optrom)(struct scsi_qla_host *, void *, @@ -3308,7 +3310,10 @@ struct isp_operations { int (*get_flash_version) (struct scsi_qla_host *, void *); int (*start_scsi) (srb_t *); int (*start_scsi_mq) (srb_t *); + + /* Context: task, might sleep */ int (*abort_isp) (struct scsi_qla_host *); + int (*iospace_config)(struct qla_hw_data *); int (*initialize_adapter)(struct scsi_qla_host *); }; diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c index b3ba0de5d4fb8..b2017f1c35044 100644 --- a/drivers/scsi/qla2xxx/qla_nx.c +++ b/drivers/scsi/qla2xxx/qla_nx.c @@ -489,29 +489,26 @@ qla82xx_rd_32(struct qla_hw_data *ha, ulong off_in) return data; } -#define IDC_LOCK_TIMEOUT 100000000 +/* + * Context: task, might sleep + */ int qla82xx_idc_lock(struct qla_hw_data *ha) { - int i; - int done = 0, timeout = 0; + const int delay_ms = 100, timeout_ms = 2000; + int done, total = 0; - while (!done) { + might_sleep(); + + while (true) { /* acquire semaphore5 from PCI HW block */ done = qla82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM5_LOCK)); if (done == 1) break; - if (timeout >= IDC_LOCK_TIMEOUT) + if (WARN_ON_ONCE(total >= timeout_ms)) return -1; - timeout++; - - /* Yield CPU */ - if (!in_interrupt()) - schedule(); - else { - for (i = 0; i < 20; i++) - cpu_relax(); - } + total += delay_ms; + msleep(delay_ms); } return 0; From patchwork Thu Nov 26 13:29:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333387 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC84DC64E7B for ; Thu, 26 Nov 2020 13:30:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 857F6221E2 for ; Thu, 26 Nov 2020 13:30:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="t2Cn8IF1"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="+pPpHOz6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390059AbgKZNaf (ORCPT ); Thu, 26 Nov 2020 08:30:35 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56528 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389961AbgKZNae (ORCPT ); Thu, 26 Nov 2020 08:30:34 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397432; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OnAEaaB4n7LL3RhdlN8tgP6NZ/XqOwpTxe9/6BlLv9M=; b=t2Cn8IF145fOAOGJ26MPtnFm6jW959s3O3r0ZKjmxMPlPysS1ZjQ/MMUxPsOzVsMc+Dqy+ Sti70TSyTI5NfCwF8yK0W4XmHWRd1dTzXw2NmmCNxLenuGjaPcpjownmx104rLXypo/oAq lIoWAA8xNmMfj6MSJJd9pdW3FNfIQZdF5Tnj+D4UtZMnKUlrCVsLfBNlrFp7sO59utCGq1 lajJDv6xwNct8HxHkD77z8qXrIujjy6yjY+NXUHoYGdvEsfAkWa4r7Lz3fKpqRiJKkNekQ 1pVIzfZirKx1w+Y7HwFObj90dhb+bNI79NBZu0A/K0b13upPXSUcLjW0RivuYw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397432; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OnAEaaB4n7LL3RhdlN8tgP6NZ/XqOwpTxe9/6BlLv9M=; b=+pPpHOz6wVDlIkqXkPyAVkpe1UqzIB14wYo4zhrjFQZI5I0zE794GlZUyJkTuvHLht263p tL1kIolpZbl3tbCw== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 05/14] scsi: qla2xxx: tcm_qla2xxx: Remove BUG_ON(in_interrupt()). Date: Thu, 26 Nov 2020 14:29:43 +0100 Message-Id: <20201126132952.2287996-6-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" tcm_qla2xxx_free_session() has a BUG_ON(in_interrupt()). While in_interrupt() is ill-defined and does not provide what the name suggests, it is not needed here: the function is always invoked from workqueue context through "struct qla_tgt_func_tmpl" ->free_session() hook it is bound to. The function also calls wait_event_timeout() down the chain, which already has a might_sleep(). Remove the in_interrupt() check. Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Nilesh Javali Cc: Reviewed-by: Daniel Wagner Reviewed-by: Himanshu Madhani --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 784b43f181818..b55fc768a2a7a 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -1400,8 +1400,6 @@ static void tcm_qla2xxx_free_session(struct fc_port *sess) struct se_session *se_sess; struct tcm_qla2xxx_lport *lport; - BUG_ON(in_interrupt()); - se_sess = sess->se_sess; if (!se_sess) { pr_err("struct fc_port->se_sess is NULL\n"); From patchwork Thu Nov 26 13:29:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333975 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5695C64E8A for ; Thu, 26 Nov 2020 13:30:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 78352221E2 for ; Thu, 26 Nov 2020 13:30:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="F1hEmG2p"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="kNZaTeKs" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390078AbgKZNaf (ORCPT ); Thu, 26 Nov 2020 08:30:35 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56550 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390051AbgKZNaf (ORCPT ); Thu, 26 Nov 2020 08:30:35 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397433; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Eq8zX1hG/IVKLKYDpk4avTvLjQi6a7SB9a8YWbl+fNg=; b=F1hEmG2pDyoBl1o+RDcGhJFdbY3FdBMLCOYEsMc/YQmVK/lC5EsLJdlR/66cXZiTj0zdUs coCH94z+zjEk/sAFy/YmsbWzStZbhdKTI8YmfotJ3udVzUcv03+pNpmBvRVjjT7Si4GkHl kB4X/6A5ZN8RjJh+y7rkyTzBehTl88fKCmteK/SccJZUeptpucF75CzqN6cQ7A6aOqoEUb sUYuFgoYEaueclQbz3OmqlZyN+o8DZSZOSJNAJJDhWm94wxchp5UgiMXLY3R9wtZszn7QO KGqRDbbq6LXEpc09h9Vc5qL02VYlf01xNWyu5hF4ivNdQpDmCTfBYI1cOgah6w== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397433; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Eq8zX1hG/IVKLKYDpk4avTvLjQi6a7SB9a8YWbl+fNg=; b=kNZaTeKsHj3jVY2UY3u55E+u+BRRThXEEuNpdZJBq99VPCJj1DRVQJTOLlWw5mGNf18DNJ Tp4J/bzWNgyEdlCw== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 06/14] scsi: qla2xxx: init/os: Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:44 +0100 Message-Id: <20201126132952.2287996-7-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" qla83xx_wait_logic() is used to control the frequency of device IDC lock retries. If in_interrupt() is true, it does 20 loops of cpu_relax(). Otherwise, it sleeps for 100ms and yields the CPU. While in_interrupt() is ill-defined and does not provide what the name suggests, it is not needed here: that qla83xx_wait_logic() is exclusively called by qla83xx_idc_lock() / unlock(), and they always run from process context. Below is an analysis of all the idc lock/unlock callers, in order of appearance: - qla_os.c: qla83xx_nic_core_unrecoverable_work(), qla83xx_idc_state_handler_work(), qla83xx_nic_core_reset_work(), qla83xx_service_idc_aen(), all workqueue context - qla_os.c: qla83xx_check_nic_core_fw_alive(), has msleep() - qla_os.c: qla83xx_set_drv_presence(), called once from qla2x00_abort_isp(), which is bound to process-context ->abort_isp() hook. It also invokes wait_for_completion_timeout() through the chain qla2x00_configure_hba() => qla24xx_link_initialize() => qla2x00_mailbox_command(). - qla_os.c: qla83xx_clear_drv_presence(), which is called from qla2x00_abort_isp() discussed above, and from qla2x00_remove_one() which is PCI process-context ->remove() hook. - qla_os.c: qla83xx_need_reset_handler(), has a one second msleep() in a loop. - qla_os.c: qla83xx_device_bootstrap(), called only by qla83xx_idc_state_handler(), which has multiple msleep() invocations. - qla_os.c: qla83xx_idc_state_handler(), multiple msleep() invocations. - qla_attr.c: qla2x00_sysfs_write_reset(), sysfs bin_attribute ->write() hook, process context - qla_init.c: qla83xx_nic_core_fw_load() => qla_init.c: qla2x00_initialize_adapter() => bound to isp_operations ->initialize_adapter() hook ** => qla_os.c: qla2x00_probe_one(), PCI ->probe() process ctx - qla_init.c: qla83xx_initiating_reset(), msleep() in a loop. - qla_init.c: qla83xx_nic_core_reset(), called by qla83xx_nic_core_reset_work(), workqueue context. Remove the in_interrupt() check, and thus replace the entirety of qla83xx_wait_logic() with an msleep(QLA83XX_WAIT_LOGIC_MS). Mark qla83xx_idc_lock() / unlock() with "Context: task, can sleep". Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Nilesh Javali Cc: GR-QLogic-Storage-Upstream@marvell.com Reviewed-by: Daniel Wagner Reviewed-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_os.c | 43 ++++++++++++++++------------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index f9c8ae9d669ef..2a8e065b192c3 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -5619,25 +5619,10 @@ qla83xx_service_idc_aen(struct work_struct *work) } } -static void -qla83xx_wait_logic(void) -{ - int i; - - /* Yield CPU */ - if (!in_interrupt()) { - /* - * Wait about 200ms before retrying again. - * This controls the number of retries for single - * lock operation. - */ - msleep(100); - schedule(); - } else { - for (i = 0; i < 20; i++) - cpu_relax(); /* This a nop instr on i386 */ - } -} +/* + * Control the frequency of IDC lock retries + */ +#define QLA83XX_WAIT_LOGIC_MS 100 static int qla83xx_force_lock_recovery(scsi_qla_host_t *base_vha) @@ -5727,7 +5712,7 @@ qla83xx_idc_lock_recovery(scsi_qla_host_t *base_vha) goto exit; if (o_drv_lockid == n_drv_lockid) { - qla83xx_wait_logic(); + msleep(QLA83XX_WAIT_LOGIC_MS); goto retry_lockid; } else return QLA_SUCCESS; @@ -5736,6 +5721,9 @@ qla83xx_idc_lock_recovery(scsi_qla_host_t *base_vha) return rval; } +/* + * Context: task, can sleep + */ void qla83xx_idc_lock(scsi_qla_host_t *base_vha, uint16_t requester_id) { @@ -5743,6 +5731,8 @@ qla83xx_idc_lock(scsi_qla_host_t *base_vha, uint16_t requester_id) uint32_t lock_owner; struct qla_hw_data *ha = base_vha->hw; + might_sleep(); + /* IDC-lock implementation using driver-lock/lock-id remote registers */ retry_lock: if (qla83xx_rd_reg(base_vha, QLA83XX_DRIVER_LOCK, &data) @@ -5761,7 +5751,7 @@ qla83xx_idc_lock(scsi_qla_host_t *base_vha, uint16_t requester_id) /* Retry/Perform IDC-Lock recovery */ if (qla83xx_idc_lock_recovery(base_vha) == QLA_SUCCESS) { - qla83xx_wait_logic(); + msleep(QLA83XX_WAIT_LOGIC_MS); goto retry_lock; } else ql_log(ql_log_warn, base_vha, 0xb075, @@ -6259,6 +6249,9 @@ void qla24xx_process_purex_list(struct purex_list *list) } } +/* + * Context: task, can sleep + */ void qla83xx_idc_unlock(scsi_qla_host_t *base_vha, uint16_t requester_id) { @@ -6269,6 +6262,8 @@ qla83xx_idc_unlock(scsi_qla_host_t *base_vha, uint16_t requester_id) uint32_t data; struct qla_hw_data *ha = base_vha->hw; + might_sleep(); + /* IDC-unlock implementation using driver-unlock/lock-id * remote registers */ @@ -6284,7 +6279,7 @@ qla83xx_idc_unlock(scsi_qla_host_t *base_vha, uint16_t requester_id) /* SV: XXX: IDC unlock retrying needed here? */ /* Retry for IDC-unlock */ - qla83xx_wait_logic(); + msleep(QLA83XX_WAIT_LOGIC_MS); retry++; ql_dbg(ql_dbg_p3p, base_vha, 0xb064, "Failed to release IDC lock, retrying=%d\n", retry); @@ -6292,7 +6287,7 @@ qla83xx_idc_unlock(scsi_qla_host_t *base_vha, uint16_t requester_id) } } else if (retry < 10) { /* Retry for IDC-unlock */ - qla83xx_wait_logic(); + msleep(QLA83XX_WAIT_LOGIC_MS); retry++; ql_dbg(ql_dbg_p3p, base_vha, 0xb065, "Failed to read drv-lockid, retrying=%d\n", retry); @@ -6308,7 +6303,7 @@ qla83xx_idc_unlock(scsi_qla_host_t *base_vha, uint16_t requester_id) if (qla83xx_access_control(base_vha, options, 0, 0, NULL)) { if (retry < 10) { /* Retry for IDC-unlock */ - qla83xx_wait_logic(); + msleep(QLA83XX_WAIT_LOGIC_MS); retry++; ql_dbg(ql_dbg_p3p, base_vha, 0xb066, "Failed to release IDC lock, retrying=%d\n", retry); From patchwork Thu Nov 26 13:29:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333973 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 233DDC71155 for ; Thu, 26 Nov 2020 13:30:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BBEE8221E9 for ; Thu, 26 Nov 2020 13:30:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="IRCzus7G"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="y1ZmSnJb" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390100AbgKZNah (ORCPT ); Thu, 26 Nov 2020 08:30:37 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56576 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390067AbgKZNag (ORCPT ); Thu, 26 Nov 2020 08:30:36 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397433; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qrQH7xIDzf20GII/6KAvMh6TDcdBZRtUy7TTmRCIPTU=; b=IRCzus7GCSHg4/0BNYEGF0nlYHnXxAbunWrghpvrKt413OjNH9K+Kr03xDPVdutfJ1ibk9 u7aNoErqz9qI2ODI+toKoP+on89iNuXgD0V6RVuZ1/sP2+ZZ8j1+/HcNsJ/rnzWkx8pvqc o6jHvetZCCTpxosc6fEdaLXKg7vkHWdNbwVQAEFUTwV9LndHcwF+oaTzS7fC2n1iM8x2lX KK/IzDsIAwy4Lc1Ha9YcZmPPB/UduOrs9wW0uBcPSVKbVGdb3mU0N6PuqytTxkviIqfCx0 TdWy+forr/6KIGf7wyCVXfwjq8IS6AsFU+ygmX8fUQ5y7+oOL5ZpmOJjVbgovg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397433; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qrQH7xIDzf20GII/6KAvMh6TDcdBZRtUy7TTmRCIPTU=; b=y1ZmSnJbXqVnwIkJoNUYZg8/VEeX8tu+Y3EBrmnoZiATjFWMCUJXgy0nP9uQSS5i0YLbsw reKqvi/RrSuyJlDA== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 07/14] scsi: qla4xxx: qla4_82xx_idc_lock(): Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:45 +0100 Message-Id: <20201126132952.2287996-8-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" qla4_82xx_idc_lock() spins on a certain hardware state until it is updated. At the end of each spin, if in_interrupt() is true, it does 20 loops of cpu_relax(). Otherwise, it yields the CPU. While in_interrupt() is ill-defined and does not provide what the name suggests, it is not needed here: qla4_82xx_idc_lock() is always called from process context. Below is an analysis of its callers: - ql4_nx.c: qla4_82xx_need_reset_handler(), 1-second msleep() in a loop. - ql4_nx.c: qla4_82xx_isp_reset(), calls qla4_8xxx_device_state_handler(), which has multiple msleep()s. Beside direct calls, qla4_82xx_idc_lock() is also bound to isp_operations ->idc_lock() hook. Other functions which are bound to the same hook, e.g. qla4_83xx_drv_lock(), also have an msleep(). For completeness, below is an analysis of all callers of that hook: - ql4_83xx.c: qla4_83xx_need_reset_handler(), has an msleep() - ql4_83xx.c: qla4_83xx_isp_reset(), calls qla4_8xxx_device_state_handler(), which has multiple msleep()s. - ql4_83xx.c: qla4_83xx_disable_pause(), all process context callers: => ql4_mbx.c: qla4xxx_mailbox_command(), msleep(), mutex_lock() => ql4_os.c: qla4xxx_recover_adapter(), schedule_timeout() in loop => ql4_os.c: qla4xxx_do_dpc(), workqueue context - ql4_attr.c: qla4_8xxx_sysfs_write_fw_dump(), sysfs bin_attribute ->write() hook, process context - ql4_mbx.c: qla4xxx_mailbox_command(), earlier discussed - ql4_nx.c: qla4_8xxx_device_bootstrap(), callers: => ql4_83xx.c: qla4_83xx_need_reset_handler(), process, msleep() => ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed - ql4_nx.c: qla4_8xxx_need_qsnt_handler(), callers: => ql4_nx.c: qla4_8xxx_device_state_handler(), multipe msleep()s => ql4_os.c: qla4xxx_do_dpc(), workqueue context - ql4_nx.c: qla4_8xxx_update_idc_reg(), callers: => ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed => ql4_os.c: qla4_8xxx_error_recovery(), only called by qla4xxx_pci_slot_reset(), which is bound to PCI ->slot_reset() process-context hook - ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed - ql4_os.c: qla4xxx_recover_adapter(), earlier discussed - ql4_os.c: qla4xxx_do_dpc(), earlier discussed Remove the in_interrupt() check. Mark, qla4_82xx_idc_lock(), and the ->idc_lock() hook itself, with "Context: task, can sleep". Change qla4_82xx_idc_lock() implementation to sleep 100ms, instead of a schedule(), for each spin. This is more deterministic, and it matches other PCI HW locking functions in the driver. Signed-off-by: Ahmed S. Darwish Cc: Nilesh Javali Cc: Manish Rangankar Cc: Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Daniel Wagner --- drivers/scsi/qla4xxx/ql4_def.h | 2 +- drivers/scsi/qla4xxx/ql4_nx.c | 14 +++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h index f5b382ed0a1b2..9210547483886 100644 --- a/drivers/scsi/qla4xxx/ql4_def.h +++ b/drivers/scsi/qla4xxx/ql4_def.h @@ -435,7 +435,7 @@ struct isp_operations { void (*wr_reg_direct) (struct scsi_qla_host *, ulong, uint32_t); int (*rd_reg_indirect) (struct scsi_qla_host *, uint32_t, uint32_t *); int (*wr_reg_indirect) (struct scsi_qla_host *, uint32_t, uint32_t); - int (*idc_lock) (struct scsi_qla_host *); + int (*idc_lock) (struct scsi_qla_host *); /* Context: task, can sleep */ void (*idc_unlock) (struct scsi_qla_host *); void (*rom_lock_recovery) (struct scsi_qla_host *); void (*queue_mailbox_command) (struct scsi_qla_host *, uint32_t *, int); diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c index da903a545b2c0..4362d0ebe0e15 100644 --- a/drivers/scsi/qla4xxx/ql4_nx.c +++ b/drivers/scsi/qla4xxx/ql4_nx.c @@ -512,12 +512,15 @@ int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data) * * General purpose lock used to synchronize access to * CRB_DEV_STATE, CRB_DEV_REF_COUNT, etc. + * + * Context: task, can sleep **/ int qla4_82xx_idc_lock(struct scsi_qla_host *ha) { - int i; int done = 0, timeout = 0; + might_sleep(); + while (!done) { /* acquire semaphore5 from PCI HW block */ done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM5_LOCK)); @@ -527,14 +530,7 @@ int qla4_82xx_idc_lock(struct scsi_qla_host *ha) return -1; timeout++; - - /* Yield CPU */ - if (!in_interrupt()) - schedule(); - else { - for (i = 0; i < 20; i++) - cpu_relax(); /*This a nop instr on i386*/ - } + msleep(100); } return 0; } From patchwork Thu Nov 26 13:29:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333976 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 74601C56202 for ; Thu, 26 Nov 2020 13:30:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 260D4221F1 for ; Thu, 26 Nov 2020 13:30:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="jNflzse8"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="rNAocpL+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390081AbgKZNag (ORCPT ); Thu, 26 Nov 2020 08:30:36 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56528 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390063AbgKZNaf (ORCPT ); Thu, 26 Nov 2020 08:30:35 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397434; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3RCu427yktHSfqwVAeE87UO1kLLTnzKktm7HKtp31hM=; b=jNflzse8IiiPcnxdY8n1k1tscAktBDx92CUgeD8b+wdnm6pbaC416l7lsDxUtWBcroaE2k vb5jxKFradW0zwV4wH2B87YsR892OrV4NZsGPAXEI1sOm49vSTlfPd+vVOMHvYrMrqyxpC fvDwAry2W1okXy59wqEbq3gIUxo3RszOjttppL84Ys5Wd6twT2AgEewDIDd3/UbuwdFbKq eXfTWpo/J/u9Xs8qvOc0JD+oIWKb7byZIMHy+Nq7K2VQFwnmh+vrjPtAitKqUKdfygTTfY HcSB+qOYgAMNXgB35HPJ+0g1mAW9mRlbDg3zRRfV885gveJ6P8jO0jhDciYGPA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397434; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3RCu427yktHSfqwVAeE87UO1kLLTnzKktm7HKtp31hM=; b=rNAocpL+hIJB3jxaPSEo/Lii/VSXzFA6Wg7EXxLe2LipUdJrpcZyyE5UGtBNYR3oxcOIaR lI1wnVGeWj66cCBQ== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 08/14] scsi: qla4xxx: qla4_82xx_rom_lock(): Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:46 +0100 Message-Id: <20201126132952.2287996-9-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" qla4_82xx_rom_lock() spins on a certain hardware state until it is updated. At the end of each spin, if in_interrupt() is true, it does 20 loops of cpu_relax(). Otherwise, it yields the CPU. While in_interrupt() is ill-defined and does not provide what the name suggests, it is not needed here: qla4_82xx_rom_lock() is always called from process context. Below is an analysis of its callers: - ql4_nx.c: qla4_82xx_rom_fast_read(), all process context callers: => ql4_nx.c: qla4_82xx_pinit_from_rom(), GFP_KERNEL allocation => ql4_nx.c: qla4_82xx_load_from_flash(), msleep() in a loop - ql4_nx.c: qla4_82xx_pinit_from_rom(), earlier discussed - ql4_nx.c: qla4_82xx_rom_lock_recovery(), bound to "isp_operations" ->rom_lock_recovery() hook, which has one process context caller, qla4_8xxx_device_bootstrap(), with callers: => ql4_83xx.c: qla4_83xx_need_reset_handler(), process, msleep() => ql4_nx.c: qla4_8xxx_device_state_handler(), multiple msleep()s - ql4_nx.c: qla4_82xx_read_flash_data(), has cond_resched() Remove the in_interrupt() check. Mark, qla4_82xx_rom_lock(), and the ->rom_lock_recovery() hook, with "Context: task, can sleep". Change qla4_82xx_rom_lock() implementation to sleep 20ms, instead of a schedule(), for each spin. This is more deterministic, and it matches the other implementations bound to ->rom_lock_recovery(). Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Nilesh Javali Cc: Manish Rangankar Cc: Reviewed-by: Daniel Wagner --- drivers/scsi/qla4xxx/ql4_def.h | 2 +- drivers/scsi/qla4xxx/ql4_nx.c | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h index 9210547483886..031569c496e57 100644 --- a/drivers/scsi/qla4xxx/ql4_def.h +++ b/drivers/scsi/qla4xxx/ql4_def.h @@ -437,7 +437,7 @@ struct isp_operations { int (*wr_reg_indirect) (struct scsi_qla_host *, uint32_t, uint32_t); int (*idc_lock) (struct scsi_qla_host *); /* Context: task, can sleep */ void (*idc_unlock) (struct scsi_qla_host *); - void (*rom_lock_recovery) (struct scsi_qla_host *); + void (*rom_lock_recovery) (struct scsi_qla_host *); /* Context: task, can sleep */ void (*queue_mailbox_command) (struct scsi_qla_host *, uint32_t *, int); void (*process_mailbox_interrupt) (struct scsi_qla_host *, int); }; diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c index 4362d0ebe0e15..fd30fbd0d33cb 100644 --- a/drivers/scsi/qla4xxx/ql4_nx.c +++ b/drivers/scsi/qla4xxx/ql4_nx.c @@ -871,15 +871,18 @@ qla4_82xx_decode_crb_addr(unsigned long addr) static long rom_max_timeout = 100; static long qla4_82xx_rom_lock_timeout = 100; +/* + * Context: task, can_sleep + */ static int qla4_82xx_rom_lock(struct scsi_qla_host *ha) { - int i; int done = 0, timeout = 0; + might_sleep(); + while (!done) { /* acquire semaphore2 from PCI HW block */ - done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM2_LOCK)); if (done == 1) break; @@ -887,14 +890,7 @@ qla4_82xx_rom_lock(struct scsi_qla_host *ha) return -1; timeout++; - - /* Yield CPU */ - if (!in_interrupt()) - schedule(); - else { - for (i = 0; i < 20; i++) - cpu_relax(); /*This a nop instr on i386*/ - } + msleep(20); } qla4_82xx_wr_32(ha, QLA82XX_ROM_LOCK_ID, ROM_LOCK_DRIVER); return 0; From patchwork Thu Nov 26 13:29:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333974 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 43D15C63697 for ; Thu, 26 Nov 2020 13:30:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E1B9721D7E for ; Thu, 26 Nov 2020 13:30:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="3ZWZnXnQ"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="D71BIfsu" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390079AbgKZNai (ORCPT ); Thu, 26 Nov 2020 08:30:38 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56550 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390051AbgKZNah (ORCPT ); Thu, 26 Nov 2020 08:30:37 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397434; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XrxBQ28uUTJB++jetuL6kEnNvt90PxZnzODz8Wc8oMw=; b=3ZWZnXnQNI3yd1oZIaL27a1gn7A8XnvQTxYuYhPvrpJW33zWzRH9N77J47ehQetaiaEIdR 7iqdkmwqeBxiHBvk5M8cYxF5mpGZacDc9w3wSJtT1wL8x31J7Ymtpd1yNDCet6FBcr3CXz cwtACi+gUxS9539GjccM6j4rwBtuSgywJhMPl17xFoabLTvX3FM/BQ5Y/jqYX9jPxAAJcs VbZCaDdx3MAwKqJNYkNFcfgdra6yVEz95LMm+36m2uHuCOhzAmg6tBCKzzgXVIrwn/kCqc lzErsCSuRMxfFzMZfOelSuzACRdmAbBgCcdQN4TygPldnI6ELRcG1tAUSOwDZQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397434; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XrxBQ28uUTJB++jetuL6kEnNvt90PxZnzODz8Wc8oMw=; b=D71BIfsun8GR62UEPsc/jExgaHeHGcRLgvZgMexCqGGsOmK4D1X7sqv2s8jAv9BMrRhdbV NNq/xxA12ymnOzBA== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 09/14] scsi: mpt3sas: Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:47 +0100 Message-Id: <20201126132952.2287996-10-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" _scsih_fw_event_cleanup_queue() waits for all outstanding firmware events wokrqueue handlers to finish. If in_interrupt() is true, it cancels itself and return early. That in_interrupt() check is ill-defined and does not provide what the name suggests: it does not cover all states in which it is safe to block and call functions like cancel_work_sync(). That check is also not needed: _scsih_fw_event_cleanup_queue() is always invoked from process context. Below is an analysis of its callers: - scsih_remove(), bound to PCI ->remove(), process context - scsih_shutdown(), bound to PCI ->shutdown(), process context - mpt3sas_scsih_clear_outstanding_scsi_tm_commands(), called by => _base_clear_outstanding_commands(), called by =>_base_fault_reset_work(), workqueue => mpt3sas_base_hard_reset_handler(), locks mutex Remove the in_interrupt() check. Change _scsih_fw_event_cleanup_queue() specification to a purely process-context function and mark it with "Context: task, can sleep". Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Sathya Prakash Cc: Sreekanth Reddy Cc: Suganath Prabu Subramani Cc: Reviewed-by: Daniel Wagner --- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index f081adb85addc..d91f45abe6b86 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -3673,6 +3673,8 @@ static struct fw_event_work *dequeue_next_fw_event(struct MPT3SAS_ADAPTER *ioc) * * Walk the firmware event queue, either killing timers, or waiting * for outstanding events to complete + * + * Context: task, can sleep */ static void _scsih_fw_event_cleanup_queue(struct MPT3SAS_ADAPTER *ioc) @@ -3680,7 +3682,7 @@ _scsih_fw_event_cleanup_queue(struct MPT3SAS_ADAPTER *ioc) struct fw_event_work *fw_event; if ((list_empty(&ioc->fw_event_list) && !ioc->current_event) || - !ioc->firmware_event_thread || in_interrupt()) + !ioc->firmware_event_thread) return; ioc->fw_events_cleanup = 1; From patchwork Thu Nov 26 13:29:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333385 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8E230C63777 for ; Thu, 26 Nov 2020 13:30:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5859721D7E for ; Thu, 26 Nov 2020 13:30:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ip+FKZTk"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="xhGeS/Ic" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390123AbgKZNai (ORCPT ); Thu, 26 Nov 2020 08:30:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56782 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390119AbgKZNai (ORCPT ); Thu, 26 Nov 2020 08:30:38 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CF7B5C0613D4 for ; Thu, 26 Nov 2020 05:30:37 -0800 (PST) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397436; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4DmSpUn3TvaIi0YjTfs82cRBGlzSa3RE3+yQ+8tMx+Q=; b=ip+FKZTkGX8NbmXLvAJYkFappUHQxrXaFJMpzAQeQsR2492Di6leYrwY8GeibG9sBC7ND0 rTQCa6NQ2ZcMA8FRAzL8xQH5cHfgrIfnWfj0sdyKbXrFZxmJU9Vs4+niNG+8NZj4nC5ULu 6FxdwKcmvDsMcr37LzYnmWAbDn+/XUaceQvWmcaIP8h8Vn6P7MnMhgjFqg7LzyYySq38uk IE4dCjbjEcqiTGVZiypa63skfGeILt06vJYD30MJx6SiwEapGBBNn0gQ2gcYMARD/cWLkF WAQNTKJJc0Wnm4HdZh7R1CnyKpceZyia8u28BtbyB0PmyMiyUSEVl5y2Xt8jXA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397436; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4DmSpUn3TvaIi0YjTfs82cRBGlzSa3RE3+yQ+8tMx+Q=; b=xhGeS/Icf3b18Dtesq1lK9vvQDd665cpSUVQ3Hvb5YcbxW/EvIlQbP3mH64Y7CmeY0r69u 7ZtlIQtUKGYKHeCg== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 10/14] scsi: myrb: Remove WARN_ON(in_interrupt()). Date: Thu, 26 Nov 2020 14:29:48 +0100 Message-Id: <20201126132952.2287996-11-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" The in_interrupt() macro is ill-defined and does not provide what the name suggests. The usage especially in driver code is deprecated and a tree-wide effort to clean up and consolidate the (ab)usage of in_interrupt() and related checks is happening. In this case the check covers only parts of the contexts in which these functions cannot be called. It fails to detect preemption or interrupt disabled invocations. As wait_for_completion() already contains a broad variety of checks (always enabled or debug option dependent) which cover all invalid conditions already, there is no point in having extra inconsistent warnings in drivers. Just remove it. Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Hannes Reinecke Reviewed-by: Daniel Wagner --- drivers/scsi/myrb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c index 5fa0f4ed6565f..3d8e91c07dc77 100644 --- a/drivers/scsi/myrb.c +++ b/drivers/scsi/myrb.c @@ -194,7 +194,6 @@ static unsigned short myrb_exec_cmd(struct myrb_hba *cb, cb->qcmd(cb, cmd_blk); spin_unlock_irqrestore(&cb->queue_lock, flags); - WARN_ON(in_interrupt()); wait_for_completion(&cmpl); return cmd_blk->status; } From patchwork Thu Nov 26 13:29:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333972 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7178EC64E7C for ; Thu, 26 Nov 2020 13:30:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 195CD21D7E for ; Thu, 26 Nov 2020 13:30:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="D9WUf0FR"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="wY4F9US2" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389937AbgKZNaj (ORCPT ); Thu, 26 Nov 2020 08:30:39 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56576 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390090AbgKZNai (ORCPT ); Thu, 26 Nov 2020 08:30:38 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397436; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=STvXLhib2DiuGdj/XJWl8ZdJjfyQ63o5jIE034MX1z8=; b=D9WUf0FRBkSPU67vTFe9SxSW4dy/pxtsAxzYsbYqUQYxo/sdg3JjFdPuk5MSLiawed+K4A VrPm4u9HPkzbclKIrAqahFYNwiF2K1Xc3qmzwywjpsubq+IwH4q1IPzjJZlq9sQG3LNCVZ OWDeVH4TZd5i6cB/Qu62Ya69Kig6lUt0EXVmG0avYT+onQH3uTstcrvUO2H8ZrrTucgahn DQAQdF5tDoAlBE9G4u5+w0yWWivro0EKyMnIKnTrsUL6y4N8VYVo6D3l3dPCHMGwcv6ejn vt6IzZzsqNssHVjzGA2a0rYVZInHOiBre3GA2NoCoyz30vUi9Y0PHaLSxzluhA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397436; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=STvXLhib2DiuGdj/XJWl8ZdJjfyQ63o5jIE034MX1z8=; b=wY4F9US2+fO340CMFDIUC4+fwTyaldHwU1WU8xCqvN431uXGAGu92DymV5qC5xcODm7DQA pqDVO5BZpnPjJzCw== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 11/14] scsi: myrs: Remove WARN_ON(in_interrupt()). Date: Thu, 26 Nov 2020 14:29:49 +0100 Message-Id: <20201126132952.2287996-12-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" The in_interrupt() macro is ill-defined and does not provide what the name suggests. The usage especially in driver code is deprecated and a tree-wide effort to clean up and consolidate the (ab)usage of in_interrupt() and related checks is happening. In this case the check covers only parts of the contexts in which these functions cannot be called. It fails to detect preemption or interrupt disabled invocations. As wait_for_completion() already contains a broad variety of checks (always enabled or debug option dependent) which cover all invalid conditions already, there is no point in having extra inconsistent warnings in drivers. Just remove it. Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Hannes Reinecke Reviewed-by: Daniel Wagner --- drivers/scsi/myrs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c index 7a3ade765ce3b..4adf9ded296aa 100644 --- a/drivers/scsi/myrs.c +++ b/drivers/scsi/myrs.c @@ -136,7 +136,6 @@ static void myrs_exec_cmd(struct myrs_hba *cs, myrs_qcmd(cs, cmd_blk); spin_unlock_irqrestore(&cs->queue_lock, flags); - WARN_ON(in_interrupt()); wait_for_completion(&complete); } From patchwork Thu Nov 26 13:29:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333383 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44991C64E8A for ; Thu, 26 Nov 2020 13:30:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E089D21D46 for ; Thu, 26 Nov 2020 13:30:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="sKTVyUux"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="We4izJOV" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390051AbgKZNao (ORCPT ); Thu, 26 Nov 2020 08:30:44 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56528 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390110AbgKZNak (ORCPT ); Thu, 26 Nov 2020 08:30:40 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397437; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qhnOM2CfVrqA4TBQ/HVK0oo98cbGQV8B2TbYW0wYyKY=; b=sKTVyUuxJqo9C6TV+M5wBxZvkuYFYG1jylAbLlzp5Wz+kya/Y+/K5K+iAt159OPEr4lKhp HazbO/U/d1ageW0OCvknpDEK2NekB+NGTlk0IMDDvym0ChJZVYtGlpFDSmFt4Y74xdRCmV WqRAy2/OpcZCgf7bZndAuPlGgyXkFWP27RsyD43+4/9m1zec3YnIsTfR7zflbfUEUmEO4F ICDoZ4yuafSk9gfqiSFlBmS/B5Vd/9hRDNYJv4LP2hWSWgjZE0apVFlTB5eK2zf1JkRame F0SyXuNfro3A3FEBu6VRm5wQDIHb1qY+6/D49F4aFKzQ9PnF7r4Jkvp8IEieMw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397437; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qhnOM2CfVrqA4TBQ/HVK0oo98cbGQV8B2TbYW0wYyKY=; b=We4izJOVUxZIi+9qI5ysH2VhQgVxXOYw0FZErkA9n3zqmzRsgQ7NREI3/DCT0UqWZUNl60 VIMs3YrVKVI4WAAA== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 12/14] scsi: NCR5380: Remove in_interrupt(). Date: Thu, 26 Nov 2020 14:29:50 +0100 Message-Id: <20201126132952.2287996-13-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: "Ahmed S. Darwish" NCR5380_poll_politely2() uses in_interrupt() to check if it is safe to sleep. The usage of in_interrupt() in drivers is phased out and Linus clearly requested that code which changes behaviour depending on context should either be separated, or the context be explicitly conveyed in an argument passed by the caller. Below is a context analysis of NCR5380_poll_politely2() uppermost callers: - NCR5380_maybe_reset_bus(), task, invoked during device probe. -> NCR5380_poll_politely() -> do_abort() - NCR5380_select(), task, but can only sleep in the "release, then re-acquire" regions of the spinlock held by its caller. Sleeping invocations (lock released): -> NCR5380_poll_politely2() Atomic invocations (lock acquired): -> NCR5380_reselect() -> NCR5380_poll_politely() -> do_abort() -> NCR5380_transfer_pio() - NCR5380_intr(), interrupt handler -> NCR5380_dma_complete() -> NCR5380_transfer_pio() -> NCR5380_poll_politely() -> NCR5380_reselect() (see above) - NCR5380_information_transfer(), task, but can only sleep in the "release, then re-acquire" regions of the caller-held spinlock. Sleeping invocations (lock released): - NCR5380_transfer_pio() -> NCR5380_poll_politely() - NCR5380_poll_politely() Atomic invocations (lock acquired): - NCR5380_transfer_dma() -> NCR5380_dma_recv_setup() => generic_NCR5380_precv() -> NCR5380_poll_politely() => macscsi_pread() -> NCR5380_poll_politely() -> NCR5380_dma_send_setup() => generic_NCR5380_psend -> NCR5380_poll_politely2() => macscsi_pwrite() -> NCR5380_poll_politely() -> NCR5380_poll_politely2() -> NCR5380_dma_complete() -> NCR5380_transfer_pio() -> NCR5380_poll_politely() - NCR5380_transfer_pio() -> NCR5380_poll_politely - NCR5380_reselect(), atomic, always called with hostdata spinlock held. If direct callers are purely atomic, or purely task context, change their specifications accordingly and mark them with "Context: " tags. For the mixed ones, trickle-down context from upper layers. Signed-off-by: Ahmed S. Darwish Signed-off-by: Sebastian Andrzej Siewior Cc: Finn Thain Cc: Michael Schmitz Cc: Acked-by: Finn Thain --- drivers/scsi/NCR5380.c | 115 ++++++++++++++++++++++----------------- drivers/scsi/NCR5380.h | 9 +-- drivers/scsi/g_NCR5380.c | 26 ++++++--- drivers/scsi/mac_scsi.c | 10 ++-- 4 files changed, 93 insertions(+), 67 deletions(-) diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c index d597d7493a627..51a80f3330faa 100644 --- a/drivers/scsi/NCR5380.c +++ b/drivers/scsi/NCR5380.c @@ -132,7 +132,7 @@ static unsigned int disconnect_mask = ~0; module_param(disconnect_mask, int, 0444); -static int do_abort(struct Scsi_Host *); +static int do_abort(struct Scsi_Host *, bool); static void do_reset(struct Scsi_Host *); static void bus_reset_cleanup(struct Scsi_Host *); @@ -198,6 +198,7 @@ static inline void set_resid_from_SCp(struct scsi_cmnd *cmd) * @bit2: Second bitmask to check * @val2: Second expected value * @wait: Time-out in jiffies + * @can_sleep: True if the function can sleep * * Polls the chip in a reasonably efficient manner waiting for an * event to occur. After a short quick poll we begin to yield the CPU @@ -210,7 +211,7 @@ static inline void set_resid_from_SCp(struct scsi_cmnd *cmd) static int NCR5380_poll_politely2(struct NCR5380_hostdata *hostdata, unsigned int reg1, u8 bit1, u8 val1, unsigned int reg2, u8 bit2, u8 val2, - unsigned long wait) + unsigned long wait, bool can_sleep) { unsigned long n = hostdata->poll_loops; unsigned long deadline = jiffies + wait; @@ -223,7 +224,7 @@ static int NCR5380_poll_politely2(struct NCR5380_hostdata *hostdata, cpu_relax(); } while (n--); - if (irqs_disabled() || in_interrupt()) + if (!can_sleep) return -ETIMEDOUT; /* Repeatedly sleep for 1 ms until deadline */ @@ -467,9 +468,10 @@ static int NCR5380_init(struct Scsi_Host *instance, int flags) * * Note that a bus reset will cause the chip to assert IRQ. * + * Context: task, can sleep + * * Returns 0 if successful, otherwise -ENXIO. */ - static int NCR5380_maybe_reset_bus(struct Scsi_Host *instance) { struct NCR5380_hostdata *hostdata = shost_priv(instance); @@ -482,11 +484,11 @@ static int NCR5380_maybe_reset_bus(struct Scsi_Host *instance) case 5: shost_printk(KERN_ERR, instance, "SCSI bus busy, waiting up to five seconds\n"); NCR5380_poll_politely(hostdata, - STATUS_REG, SR_BSY, 0, 5 * HZ); + STATUS_REG, SR_BSY, 0, 5 * HZ, true); break; case 2: shost_printk(KERN_ERR, instance, "bus busy, attempting abort\n"); - do_abort(instance); + do_abort(instance, true); break; case 4: shost_printk(KERN_ERR, instance, "bus busy, attempting reset\n"); @@ -690,7 +692,6 @@ static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd) * NCR5380_queue_command() and NCR5380_intr() will try to start it * in case it is not running. */ - static void NCR5380_main(struct work_struct *work) { struct NCR5380_hostdata *hostdata = @@ -750,10 +751,9 @@ static void NCR5380_main(struct work_struct *work) * NCR5380_dma_complete - finish DMA transfer * @instance: the scsi host instance * - * Called by the interrupt handler when DMA finishes or a phase - * mismatch occurs (which would end the DMA transfer). + * Context: atomic. Called by the interrupt handler when DMA finishes + * or a phase mismatch occurs (which would end the DMA transfer). */ - static void NCR5380_dma_complete(struct Scsi_Host *instance) { struct NCR5380_hostdata *hostdata = shost_priv(instance); @@ -822,7 +822,7 @@ static void NCR5380_dma_complete(struct Scsi_Host *instance) if (toPIO > 0) { dsprintk(NDEBUG_DMA, instance, "Doing %d byte PIO to 0x%p\n", cnt, *data); - NCR5380_transfer_pio(instance, &p, &cnt, data); + NCR5380_transfer_pio(instance, &p, &cnt, data, false); *count -= toPIO - cnt; } } @@ -962,8 +962,11 @@ static irqreturn_t __maybe_unused NCR5380_intr(int irq, void *dev_id) * * If failed (no target) : cmd->scsi_done() will be called, and the * cmd->result host byte set to DID_BAD_TARGET. + * + * Context: task context, with @instance hostdata spinlock held by + * caller. That is, this function can sleep in the areas between + * releasing and re-acquring that spinlock. */ - static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) __releases(&hostdata->lock) __acquires(&hostdata->lock) { @@ -1010,8 +1013,10 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) spin_unlock_irq(&hostdata->lock); err = NCR5380_poll_politely2(hostdata, MODE_REG, MR_ARBITRATE, 0, - INITIATOR_COMMAND_REG, ICR_ARBITRATION_PROGRESS, - ICR_ARBITRATION_PROGRESS, HZ); + INITIATOR_COMMAND_REG, + ICR_ARBITRATION_PROGRESS, + ICR_ARBITRATION_PROGRESS, + HZ, true); spin_lock_irq(&hostdata->lock); if (!(NCR5380_read(MODE_REG) & MR_ARBITRATE)) { /* Reselection interrupt */ @@ -1136,7 +1141,7 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) */ err = NCR5380_poll_politely(hostdata, STATUS_REG, SR_BSY, SR_BSY, - msecs_to_jiffies(250)); + msecs_to_jiffies(250), true); if ((NCR5380_read(STATUS_REG) & (SR_SEL | SR_IO)) == (SR_SEL | SR_IO)) { spin_lock_irq(&hostdata->lock); @@ -1181,7 +1186,7 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) /* Wait for start of REQ/ACK handshake */ - err = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ); + err = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ, true); spin_lock_irq(&hostdata->lock); if (err < 0) { shost_printk(KERN_ERR, instance, "select: REQ timeout\n"); @@ -1189,7 +1194,7 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) goto out; } if (!hostdata->selecting) { - do_abort(instance); + do_abort(instance, false); return false; } @@ -1200,7 +1205,7 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) len = 1; data = tmp; phase = PHASE_MSGOUT; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); if (len) { NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE); cmd->result = DID_ERROR << 16; @@ -1238,7 +1243,8 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) * * Inputs : instance - instance of driver, *phase - pointer to * what phase is expected, *count - pointer to number of - * bytes to transfer, **data - pointer to data pointer. + * bytes to transfer, **data - pointer to data pointer, + * can_sleep - whether it is safe to sleep. * * Returns : -1 when different phase is entered without transferring * maximum number of bytes, 0 if all bytes are transferred or exit @@ -1257,7 +1263,7 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd) static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, - unsigned char **data) + unsigned char **data, bool can_sleep) { struct NCR5380_hostdata *hostdata = shost_priv(instance); unsigned char p = *phase, tmp; @@ -1278,7 +1284,8 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance, * valid */ - if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ) < 0) + if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, + HZ, can_sleep) < 0) break; dsprintk(NDEBUG_HANDSHAKE, instance, "REQ asserted\n"); @@ -1324,7 +1331,7 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance, } if (NCR5380_poll_politely(hostdata, - STATUS_REG, SR_REQ, 0, 5 * HZ) < 0) + STATUS_REG, SR_REQ, 0, 5 * HZ, can_sleep) < 0) break; dsprintk(NDEBUG_HANDSHAKE, instance, "REQ negated, handshake complete\n"); @@ -1399,11 +1406,12 @@ static void do_reset(struct Scsi_Host *instance) * do_abort - abort the currently established nexus by going to * MESSAGE OUT phase and sending an ABORT message. * @instance: relevant scsi host instance + * @can_sleep: true if the function can sleep * * Returns 0 on success, negative error code on failure. */ -static int do_abort(struct Scsi_Host *instance) +static int do_abort(struct Scsi_Host *instance, bool can_sleep) { struct NCR5380_hostdata *hostdata = shost_priv(instance); unsigned char *msgptr, phase, tmp; @@ -1423,7 +1431,8 @@ static int do_abort(struct Scsi_Host *instance) * the target sees, so we just handshake. */ - rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, 10 * HZ); + rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, + 10 * HZ, can_sleep); if (rc < 0) goto out; @@ -1434,7 +1443,8 @@ static int do_abort(struct Scsi_Host *instance) if (tmp != PHASE_MSGOUT) { NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_ATN | ICR_ASSERT_ACK); - rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, 0, 3 * HZ); + rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, 0, + 3 * HZ, can_sleep); if (rc < 0) goto out; NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_ATN); @@ -1444,7 +1454,7 @@ static int do_abort(struct Scsi_Host *instance) msgptr = &tmp; len = 1; phase = PHASE_MSGOUT; - NCR5380_transfer_pio(instance, &phase, &len, &msgptr); + NCR5380_transfer_pio(instance, &phase, &len, &msgptr, can_sleep); if (len) rc = -ENXIO; @@ -1474,9 +1484,9 @@ static int do_abort(struct Scsi_Host *instance) * is in same phase. * * Also, *phase, *count, *data are modified in place. + * + * Context: atomic, @instance hostdata spinlock held */ - - static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data) @@ -1623,12 +1633,12 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance, */ if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, - BASR_DRQ, BASR_DRQ, HZ) < 0) { + BASR_DRQ, BASR_DRQ, HZ, false) < 0) { result = -1; shost_printk(KERN_ERR, instance, "PDMA read: DRQ timeout\n"); } if (NCR5380_poll_politely(hostdata, STATUS_REG, - SR_REQ, 0, HZ) < 0) { + SR_REQ, 0, HZ, false) < 0) { result = -1; shost_printk(KERN_ERR, instance, "PDMA read: !REQ timeout\n"); } @@ -1640,7 +1650,7 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance, */ if (NCR5380_poll_politely2(hostdata, BUS_AND_STATUS_REG, BASR_DRQ, BASR_DRQ, - BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, HZ) < 0) { + BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, HZ, false) < 0) { result = -1; shost_printk(KERN_ERR, instance, "PDMA write: DRQ and phase timeout\n"); } @@ -1666,8 +1676,11 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance, * * XXX Note : we need to watch for bus free or a reset condition here * to recover from an unexpected bus free condition. + * + * Context: task context, with @instance hostdata spinlock held by + * caller. That is, this function can sleep in the areas between + * releasing and re-acquring that spinlock. */ - static void NCR5380_information_transfer(struct Scsi_Host *instance) __releases(&hostdata->lock) __acquires(&hostdata->lock) { @@ -1737,7 +1750,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) #if (NDEBUG & NDEBUG_NO_DATAOUT) shost_printk(KERN_DEBUG, instance, "NDEBUG_NO_DATAOUT set, attempted DATAOUT aborted\n"); sink = 1; - do_abort(instance); + do_abort(instance, false); cmd->result = DID_ERROR << 16; complete_cmd(instance, cmd); hostdata->connected = NULL; @@ -1793,7 +1806,8 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) NCR5380_PIO_CHUNK_SIZE); len = transfersize; NCR5380_transfer_pio(instance, &phase, &len, - (unsigned char **)&cmd->SCp.ptr); + (unsigned char **)&cmd->SCp.ptr, + false); cmd->SCp.this_residual -= transfersize - len; } #ifdef CONFIG_SUN3 @@ -1804,7 +1818,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) case PHASE_MSGIN: len = 1; data = &tmp; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); cmd->SCp.Message = tmp; switch (tmp) { @@ -1910,7 +1924,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) len = 2; data = extended_msg + 1; phase = PHASE_MSGIN; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, true); dsprintk(NDEBUG_EXTENDED, instance, "length %d, code 0x%02x\n", (int)extended_msg[1], (int)extended_msg[2]); @@ -1923,7 +1937,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) data = extended_msg + 3; phase = PHASE_MSGIN; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, true); dsprintk(NDEBUG_EXTENDED, instance, "message received, residual %d\n", len); @@ -1970,7 +1984,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) len = 1; data = &msgout; hostdata->last_message = msgout; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); if (msgout == ABORT) { hostdata->connected = NULL; hostdata->busy[scmd_id(cmd)] &= ~(1 << cmd->device->lun); @@ -1988,12 +2002,12 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) * PSEUDO-DMA architecture we should probably * use the dma transfer function. */ - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); break; case PHASE_STATIN: len = 1; data = &tmp; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); cmd->SCp.Status = tmp; break; default: @@ -2002,7 +2016,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) } /* switch(phase) */ } else { spin_unlock_irq(&hostdata->lock); - NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ); + NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ, true); spin_lock_irq(&hostdata->lock); } } @@ -2016,8 +2030,9 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) * nexus has been reestablished, * * Inputs : instance - this instance of the NCR5380. + * + * Context: atomic, with @instance hostdata spinlock held */ - static void NCR5380_reselect(struct Scsi_Host *instance) { struct NCR5380_hostdata *hostdata = shost_priv(instance); @@ -2052,7 +2067,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance) NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_BSY); if (NCR5380_poll_politely(hostdata, - STATUS_REG, SR_SEL, 0, 2 * HZ) < 0) { + STATUS_REG, SR_SEL, 0, 2 * HZ, false) < 0) { shost_printk(KERN_ERR, instance, "reselect: !SEL timeout\n"); NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE); return; @@ -2064,12 +2079,12 @@ static void NCR5380_reselect(struct Scsi_Host *instance) */ if (NCR5380_poll_politely(hostdata, - STATUS_REG, SR_REQ, SR_REQ, 2 * HZ) < 0) { + STATUS_REG, SR_REQ, SR_REQ, 2 * HZ, false) < 0) { if ((NCR5380_read(STATUS_REG) & (SR_BSY | SR_SEL)) == 0) /* BUS FREE phase */ return; shost_printk(KERN_ERR, instance, "reselect: REQ timeout\n"); - do_abort(instance); + do_abort(instance, false); return; } @@ -2085,10 +2100,10 @@ static void NCR5380_reselect(struct Scsi_Host *instance) unsigned char *data = msg; unsigned char phase = PHASE_MSGIN; - NCR5380_transfer_pio(instance, &phase, &len, &data); + NCR5380_transfer_pio(instance, &phase, &len, &data, false); if (len) { - do_abort(instance); + do_abort(instance, false); return; } } @@ -2098,7 +2113,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance) shost_printk(KERN_ERR, instance, "expecting IDENTIFY message, got "); spi_print_msg(msg); printk("\n"); - do_abort(instance); + do_abort(instance, false); return; } lun = msg[0] & 0x07; @@ -2138,7 +2153,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance) * Since we have an established nexus that we can't do anything * with, we must abort it. */ - if (do_abort(instance) == 0) + if (do_abort(instance, false) == 0) hostdata->busy[target] &= ~(1 << lun); return; } @@ -2285,7 +2300,7 @@ static int NCR5380_abort(struct scsi_cmnd *cmd) dsprintk(NDEBUG_ABORT, instance, "abort: cmd %p is connected\n", cmd); hostdata->connected = NULL; hostdata->dma_len = 0; - if (do_abort(instance) < 0) { + if (do_abort(instance, false) < 0) { set_host_byte(cmd, DID_ERROR); complete_cmd(instance, cmd); result = FAILED; diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h index 5935fd6d1a058..7c569e92ca58e 100644 --- a/drivers/scsi/NCR5380.h +++ b/drivers/scsi/NCR5380.h @@ -277,20 +277,21 @@ static const char *NCR5380_info(struct Scsi_Host *instance); static void NCR5380_reselect(struct Scsi_Host *instance); static bool NCR5380_select(struct Scsi_Host *, struct scsi_cmnd *); static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data); -static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data); +static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, + int *count, unsigned char **data, bool can_sleep); static int NCR5380_poll_politely2(struct NCR5380_hostdata *, unsigned int, u8, u8, - unsigned int, u8, u8, unsigned long); + unsigned int, u8, u8, unsigned long, bool); static inline int NCR5380_poll_politely(struct NCR5380_hostdata *hostdata, unsigned int reg, u8 bit, u8 val, - unsigned long wait) + unsigned long wait, bool can_sleep) { if ((NCR5380_read(reg) & bit) == val) return 0; return NCR5380_poll_politely2(hostdata, reg, bit, val, - reg, bit, val, wait); + reg, bit, val, wait, can_sleep); } static int NCR5380_dma_xfer_len(struct NCR5380_hostdata *, diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 29e4cdcade720..06b1fcfd33cc9 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -513,9 +513,11 @@ static void wait_for_53c80_access(struct NCR5380_hostdata *hostdata) * @dst: buffer to write into * @len: transfer size * + * Context: atomic. This implements NCR5380.c NCR5380_dma_recv_setup(), + * which is always called with @hostdata spinlock held. + * * Perform a pseudo DMA mode receive from a 53C400 or equivalent device. */ - static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata, unsigned char *dst, int len) { @@ -529,14 +531,16 @@ static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata, if (start == len - 128) { /* Ignore End of DMA interrupt for the final buffer */ if (NCR5380_poll_politely(hostdata, hostdata->c400_ctl_status, - CSR_HOST_BUF_NOT_RDY, 0, HZ / 64) < 0) + CSR_HOST_BUF_NOT_RDY, 0, HZ / 64, + false) < 0) break; } else { if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status, CSR_HOST_BUF_NOT_RDY, 0, hostdata->c400_ctl_status, CSR_GATED_53C80_IRQ, - CSR_GATED_53C80_IRQ, HZ / 64) < 0 || + CSR_GATED_53C80_IRQ, HZ / 64, + false) < 0 || NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) break; } @@ -565,7 +569,8 @@ static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata, if (residual == 0 && NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, BASR_END_DMA_TRANSFER, BASR_END_DMA_TRANSFER, - HZ / 64) < 0) + HZ / 64, + false) < 0) scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout\n", __func__); @@ -580,9 +585,11 @@ static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata, * @src: buffer to read from * @len: transfer size * + * Context: atomic. This implements NCR5380.c NCR5380_dma_send_setup(), + * which is always called with @hostdata spinlock held. + * * Perform a pseudo DMA mode send to a 53C400 or equivalent device. */ - static inline int generic_NCR5380_psend(struct NCR5380_hostdata *hostdata, unsigned char *src, int len) { @@ -597,7 +604,8 @@ static inline int generic_NCR5380_psend(struct NCR5380_hostdata *hostdata, CSR_HOST_BUF_NOT_RDY, 0, hostdata->c400_ctl_status, CSR_GATED_53C80_IRQ, - CSR_GATED_53C80_IRQ, HZ / 64) < 0 || + CSR_GATED_53C80_IRQ, HZ / 64, + false) < 0 || NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) { /* Both 128 B buffers are in use */ if (start >= 128) @@ -644,13 +652,15 @@ static inline int generic_NCR5380_psend(struct NCR5380_hostdata *hostdata, if (residual == 0) { if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG, TCR_LAST_BYTE_SENT, TCR_LAST_BYTE_SENT, - HZ / 64) < 0) + HZ / 64, + false) < 0) scmd_printk(KERN_ERR, hostdata->connected, "%s: Last Byte Sent timeout\n", __func__); if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, BASR_END_DMA_TRANSFER, BASR_END_DMA_TRANSFER, - HZ / 64) < 0) + HZ / 64, + false) < 0) scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout\n", __func__); } diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c index b5dde9d0d0545..3c39db74fd847 100644 --- a/drivers/scsi/mac_scsi.c +++ b/drivers/scsi/mac_scsi.c @@ -285,7 +285,7 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata, while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, BASR_DRQ | BASR_PHASE_MATCH, - BASR_DRQ | BASR_PHASE_MATCH, HZ / 64)) { + BASR_DRQ | BASR_PHASE_MATCH, HZ / 64, false)) { int bytes; if (macintosh_config->ident == MAC_MODEL_IIFX) @@ -304,7 +304,7 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata, if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ, BUS_AND_STATUS_REG, BASR_ACK, - BASR_ACK, HZ / 64) < 0) + BASR_ACK, HZ / 64, false) < 0) scmd_printk(KERN_DEBUG, hostdata->connected, "%s: !REQ and !ACK\n", __func__); if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) @@ -344,7 +344,7 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata, while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, BASR_DRQ | BASR_PHASE_MATCH, - BASR_DRQ | BASR_PHASE_MATCH, HZ / 64)) { + BASR_DRQ | BASR_PHASE_MATCH, HZ / 64, false)) { int bytes; if (macintosh_config->ident == MAC_MODEL_IIFX) @@ -362,7 +362,7 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata, if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG, TCR_LAST_BYTE_SENT, TCR_LAST_BYTE_SENT, - HZ / 64) < 0) { + HZ / 64, false) < 0) { scmd_printk(KERN_ERR, hostdata->connected, "%s: Last Byte Sent timeout\n", __func__); result = -1; @@ -372,7 +372,7 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata, if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ, BUS_AND_STATUS_REG, BASR_ACK, - BASR_ACK, HZ / 64) < 0) + BASR_ACK, HZ / 64, false) < 0) scmd_printk(KERN_DEBUG, hostdata->connected, "%s: !REQ and !ACK\n", __func__); if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) From patchwork Thu Nov 26 13:29:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 333384 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2545BC64E7D for ; Thu, 26 Nov 2020 13:30:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC29E21D40 for ; Thu, 26 Nov 2020 13:30:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="UVx+cuii"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="mpfunGzg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389945AbgKZNan (ORCPT ); Thu, 26 Nov 2020 08:30:43 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:56576 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390124AbgKZNak (ORCPT ); Thu, 26 Nov 2020 08:30:40 -0500 From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1606397438; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xOOBp6z3OInykv2pKK7i4G7TCjhXQtxisN5E2TisB+U=; b=UVx+cuiijhsjFzna9+6Zqbcckv8jr+L/hm8tSPZxe/bQJNiWItdNPTm4uLgd0T6p2PiU1u 1Fj9u2FOn3UCQXAt0HC3Cz7eWVIldXES05AohHDsrP9G97NGioK8VESTGxHMOn+aDDIvOs SdyCLd7KoGxDk6eJU7MbbTCMqc1YIfxN3prsAP8tcFnTxi3YDqaBaKhY/TCO2fA+taJ/8K uYf6baBKGsUKh33aVbKJsMTEX4LpV71Fnjm7YJyiOxaTUkPGMw7Zw3fHAlJho3JOndCjJB 7yUv1gpXkbeplah/1Er3WVqMLWz/X/TlymvPxv/UGDarmpijcSx6iBmEyWc/UQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1606397438; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xOOBp6z3OInykv2pKK7i4G7TCjhXQtxisN5E2TisB+U=; b=mpfunGzghZF4WsR3K6MnZfzrV/WdyF3J1uNBm0rLDM4bup9qGPN8JK7MbEdo+da6w+Mv/K VkmQ5T22WC26VCCw== To: linux-scsi@vger.kernel.org Cc: Finn Thain , GR-QLogic-Storage-Upstream@marvell.com, Hannes Reinecke , Jack Wang , John Garry , linux-m68k@lists.linux-m68k.org, Manish Rangankar , Michael Schmitz , MPT-FusionLinux.pdl@broadcom.com, Nilesh Javali , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Vikram Auradkar , Xiang Chen , Xiaofei Tan , "James E . J . Bottomley" , "Martin K . Petersen" , Thomas Gleixner , "Ahmed S . Darwish" , Sebastian Andrzej Siewior Subject: [PATCH 14/14] scsi: message: fusion: Remove in_interrupt() usage in mptsas_cleanup_fw_event_q(). Date: Thu, 26 Nov 2020 14:29:52 +0100 Message-Id: <20201126132952.2287996-15-bigeasy@linutronix.de> In-Reply-To: <20201126132952.2287996-1-bigeasy@linutronix.de> References: <20201126132952.2287996-1-bigeasy@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org mptsas_cleanup_fw_event_q() uses in_interrupt() to determine if it is safe to cancel a worker item. Aside of that in_interrupt() is deprecated as it does not provide what the name suggests. It covers more than hard/soft interrupt servicing context and is semantically ill defined. Looking closer there are a few problems with the current construct: - It could be invoked from an interrupt handler / non-blocking context because cancel_delayed_work() has no such restriction. Also, mptsas_free_fw_event() has no such restriction. - The list is accessed unlocked. It may dequeue a valid work-item but at the time of invoking cancel_delayed_work() the memory may be released or reused because the worker has already run. mptsas_cleanup_fw_event_q() is invoked via mptsas_shutdown() which is always invoked from preemtible context on device shutdown. It is also invoked via mptsas_ioc_reset(, MPT_IOC_POST_RESET) which is a MptResetHandlers callback. The only caller here are mpt_SoftResetHandler(), mpt_HardResetHandler() and mpt_Soft_Hard_ResetHandler(). All these functions have a `sleepFlag' argument and each caller uses caller uses `CAN_SLEEP' here and according to current documentation: | @sleepFlag: Indicates if sleep or schedule must be called So it is safe to sleep. Add mptsas_hotplug_event::users member. Initialize it to one by default so mptsas_free_fw_event() will free the memory. mptsas_cleanup_fw_event_q() will increment its value for items it dequeues and then it may keep a pointer after dropping the lock. Invoke cancel_delayed_work_sync() to cancel the work item and wait if the worker is currently busy. Free the memory afterwards since it owns the last reference to it. Signed-off-by: Sebastian Andrzej Siewior Cc: Sathya Prakash Cc: Sreekanth Reddy Cc: Suganath Prabu Subramani Cc: MPT-FusionLinux.pdl@broadcom.com Reviewed-by: Daniel Wagner --- drivers/message/fusion/mptsas.c | 45 +++++++++++++++++++++++++-------- drivers/message/fusion/mptsas.h | 1 + 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c index 18b91ea1a353f..5eb0b3361e4e0 100644 --- a/drivers/message/fusion/mptsas.c +++ b/drivers/message/fusion/mptsas.c @@ -289,6 +289,7 @@ mptsas_add_fw_event(MPT_ADAPTER *ioc, struct fw_event_work *fw_event, spin_lock_irqsave(&ioc->fw_event_lock, flags); list_add_tail(&fw_event->list, &ioc->fw_event_list); + fw_event->users = 1; INIT_DELAYED_WORK(&fw_event->work, mptsas_firmware_event_work); devtprintk(ioc, printk(MYIOC_s_DEBUG_FMT "%s: add (fw_event=0x%p)" "on cpuid %d\n", ioc->name, __func__, @@ -314,6 +315,15 @@ mptsas_requeue_fw_event(MPT_ADAPTER *ioc, struct fw_event_work *fw_event, spin_unlock_irqrestore(&ioc->fw_event_lock, flags); } +static void __mptsas_free_fw_event(MPT_ADAPTER *ioc, + struct fw_event_work *fw_event) +{ + devtprintk(ioc, printk(MYIOC_s_DEBUG_FMT "%s: kfree (fw_event=0x%p)\n", + ioc->name, __func__, fw_event)); + list_del(&fw_event->list); + kfree(fw_event); +} + /* free memory associated to a sas firmware event */ static void mptsas_free_fw_event(MPT_ADAPTER *ioc, struct fw_event_work *fw_event) @@ -321,10 +331,9 @@ mptsas_free_fw_event(MPT_ADAPTER *ioc, struct fw_event_work *fw_event) unsigned long flags; spin_lock_irqsave(&ioc->fw_event_lock, flags); - devtprintk(ioc, printk(MYIOC_s_DEBUG_FMT "%s: kfree (fw_event=0x%p)\n", - ioc->name, __func__, fw_event)); - list_del(&fw_event->list); - kfree(fw_event); + fw_event->users--; + if (!fw_event->users) + __mptsas_free_fw_event(ioc, fw_event); spin_unlock_irqrestore(&ioc->fw_event_lock, flags); } @@ -333,9 +342,10 @@ mptsas_free_fw_event(MPT_ADAPTER *ioc, struct fw_event_work *fw_event) static void mptsas_cleanup_fw_event_q(MPT_ADAPTER *ioc) { - struct fw_event_work *fw_event, *next; + struct fw_event_work *fw_event; struct mptsas_target_reset_event *target_reset_list, *n; MPT_SCSI_HOST *hd = shost_priv(ioc->sh); + unsigned long flags; /* flush the target_reset_list */ if (!list_empty(&hd->target_reset_list)) { @@ -350,14 +360,29 @@ mptsas_cleanup_fw_event_q(MPT_ADAPTER *ioc) } } - if (list_empty(&ioc->fw_event_list) || - !ioc->fw_event_q || in_interrupt()) + if (list_empty(&ioc->fw_event_list) || !ioc->fw_event_q) return; - list_for_each_entry_safe(fw_event, next, &ioc->fw_event_list, list) { - if (cancel_delayed_work(&fw_event->work)) - mptsas_free_fw_event(ioc, fw_event); + spin_lock_irqsave(&ioc->fw_event_lock, flags); + + while (!list_empty(&ioc->fw_event_list)) { + bool canceled = false; + + fw_event = list_first_entry(&ioc->fw_event_list, + struct fw_event_work, list); + fw_event->users++; + spin_unlock_irqrestore(&ioc->fw_event_lock, flags); + if (cancel_delayed_work_sync(&fw_event->work)) + canceled = true; + + spin_lock_irqsave(&ioc->fw_event_lock, flags); + if (canceled) + fw_event->users--; + fw_event->users--; + WARN_ON_ONCE(fw_event->users); + __mptsas_free_fw_event(ioc, fw_event); } + spin_unlock_irqrestore(&ioc->fw_event_lock, flags); } diff --git a/drivers/message/fusion/mptsas.h b/drivers/message/fusion/mptsas.h index e35b13891fe42..71abf3477495e 100644 --- a/drivers/message/fusion/mptsas.h +++ b/drivers/message/fusion/mptsas.h @@ -107,6 +107,7 @@ struct mptsas_hotplug_event { struct fw_event_work { struct list_head list; struct delayed_work work; + int users; MPT_ADAPTER *ioc; u32 event; u8 retries;